Implement propagation of custom tags to all test event levels#11366
Implement propagation of custom tags to all test event levels#11366daniel-mohedano wants to merge 3 commits into
Conversation
daniel-mohedano
added
type: enhancement
daniel-mohedano
requested review from
manuel-alvarez-alvarez and
sarahchen6
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fec4d9d65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Mocks AgentSpan (interface) rather than DDSpan because propagateCustomTags writes through | ||
| // the final DDSpan#setTag(String, String) overload, which Spock cannot intercept on a class mock. | ||
| def "test custom tag propagation from span: child=#childValue, parent=#parentValue, key=#key, allowlist=#allowlist"() { |
There was a problem hiding this comment.
AGENTS.md for this repo says, “Do not write new Groovy / Spock tests and migrate the existing one to JUnit 5 if it is written in Groovy.” The new custom-tag propagation coverage added here extends a Spock spec, so this change violates the repository’s test convention; please move these cases to a JUnit 5 test or migrate the existing spec instead of adding more Spock coverage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Although it's not a new test, we'll have to do the migration at some point, so migrated the test file to JUnit 5 in eca6ec9
| "DD_CIVISIBILITY_PROPAGATED_TAGS": [ | ||
| { | ||
| "version": "A", | ||
| "type": "string", | ||
| "default": null, | ||
| "aliases": [] | ||
| } | ||
| ], |
There was a problem hiding this comment.
Waiting on updating the configuration registry until the PR is ready to be merged 🚀
|
Indeed, after some more testing they do seem to be handled differently, while still allowing some numerical-like operations even with the strings. Addressed in 3d5e279 to keep the typing of tags |
sarahchen6
left a comment
sarahchen6
left a comment
There was a problem hiding this comment.
Looks good from LP-side (after configs are updated)!
3 participants
What Does This Do
DD_CIVISIBILITY_PROPAGATED_TAGS(e.g.DD_CIVISIBILITY_PROPAGATED_TAGS=custom.shard_index,custom.total_shards,build.id)Motivation
These changes allow users to programmatically set custom tags at session and module level, which was previously not possible, by setting them at test (or suite) level.
Additional Notes
test-environment-trigger: skip
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.