Truncate CI Visibility meta tag values#11485
Conversation
|
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
![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: b200a6d0b5
ℹ️ 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".
| /* 2,1,1 */ | ||
| headerWriter.writeUTF8(ENV); | ||
| headerWriter.writeUTF8(wellKnownTags.getEnv()); | ||
| headerWriter.writeUTF8(truncate(wellKnownTags.getEnv())); |
There was a problem hiding this comment.
In this header path, CiVisibilityWellKnownTags#getEnv() and the following well-known tag getters return UTF8BytesString, but the new truncate helper only accepts String. Java has to resolve truncate(...) before it can call writeUTF8, so this makes dd-trace-core fail to compile for the CI test-cycle mapper; convert these values to strings first or make the helper accept CharSequence.
Useful? React with 👍 / 👎.
| assert spanContent.containsKey("parent_id") | ||
| } | ||
|
|
||
| def "truncates meta string values and preserves metrics and top level ids"() { |
There was a problem hiding this comment.
This adds new Spock/Groovy coverage in a file under /workspace/dd-trace-java, but the repository instructions in /workspace/dd-trace-java/AGENTS.md explicitly say not to write new Groovy/Spock tests and to migrate existing Groovy tests to JUnit 5. Please move this new coverage to a JUnit 5 Java test instead of expanding the Spock suite.
Useful? React with 👍 / 👎.
| /* 2,1,1 */ | ||
| headerWriter.writeUTF8(ENV); | ||
| headerWriter.writeUTF8(wellKnownTags.getEnv()); | ||
| headerWriter.writeUTF8(truncate(wellKnownTags.getEnv())); |
There was a problem hiding this comment.
For well-known tags that will never change I'd recommend creating your own CI-Vis holder class containing truncated well-known tags. That would be more efficient and could also help address the comment above, because you could ensure that any truncated well-known tag values stay as the more efficient UTF8BytesString for marshalling purposes.
I'd also suggest looking at the existing Strings utility class that has truncate methods for String and CharSequence. You could add a similar truncate utility method that accepts UTF8BytesString and returns a UTF8BytesString.
daniel-mohedano
added
comp: ci visibility
3 participants
What Does This Do
Truncates CI Visibility metadata/tag string values to 5000 characters before msgpack encoding and file-based intake dispatch.
Motivation
The backend maximum allowed tag value size is 5000 characters. Truncating in the tracer prevents oversized metadata values from being sent in CI Visibility payloads.
Additional Notes
Non-string metadata values are preserved as-is. Added unit coverage for both the CI test-cycle mapper path and file-based payload dispatcher path.
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 issue/merge.Jira ticket: N/A
Validation:
git diff --checkThe Java test task was not run locally because this machine does not currently have a Java runtime available.