fix(profiling): missing headers in exporter #1493
Open
+106
−41
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📚 Documentation Check Results📦
|
🔒 Cargo Deny Results📦
|
BenchmarksComparisonBenchmark execution time: 2026-01-30 02:45:28 Comparing candidate commit e677049 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 57 metrics, 2 unstable metrics. CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
Group 13
Group 14
Group 15
Group 16
Group 17
Group 18
Group 19
BaselineOmitted due to size. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1493 +/- ##
==========================================
- Coverage 71.04% 71.02% -0.02%
==========================================
Files 422 422
Lines 68697 68699 +2
==========================================
- Hits 48804 48793 -11
- Misses 19893 19906 +13
🚀 New features to boost your workflow:
|
ivoanjo
approved these changes
Jan 30, 2026
Member
ivoanjo
left a comment
ivoanjo
left a comment
There was a problem hiding this comment.
👍 LGTM, Added a note about testing
Comment on lines
+10
to
+56
| pub fn assert_entity_headers_match(headers: &HashMap<String, String>) { | ||
| // Check for entity headers and validate their values match what libdd_common provides | ||
| let expected_container_id = libdd_common::entity_id::get_container_id(); | ||
| let expected_entity_id = libdd_common::entity_id::get_entity_id(); | ||
| let expected_external_env = *libdd_common::entity_id::DD_EXTERNAL_ENV; | ||
|
|
||
| // Validate container ID | ||
| if let Some(expected) = expected_container_id { | ||
| assert_eq!( | ||
| headers.get("datadog-container-id"), | ||
| Some(&expected.to_string()), | ||
| "datadog-container-id header should match the value from entity_id::get_container_id()" | ||
| ); | ||
| } else { | ||
| assert!( | ||
| !headers.contains_key("datadog-container-id"), | ||
| "datadog-container-id header should not be present when entity_id::get_container_id() is None" | ||
| ); | ||
| } | ||
|
|
||
| // Validate entity ID | ||
| if let Some(expected) = expected_entity_id { | ||
| assert_eq!( | ||
| headers.get("datadog-entity-id"), | ||
| Some(&expected.to_string()), | ||
| "datadog-entity-id header should match the value from entity_id::get_entity_id()" | ||
| ); | ||
| } else { | ||
| assert!( | ||
| !headers.contains_key("datadog-entity-id"), | ||
| "datadog-entity-id header should not be present when entity_id::get_entity_id() is None" | ||
| ); | ||
| } | ||
|
|
||
| // Validate external env | ||
| if let Some(expected) = expected_external_env { | ||
| assert_eq!( | ||
| headers.get("datadog-external-env"), | ||
| Some(&expected.to_string()), | ||
| "datadog-external-env header should match the value from entity_id::DD_EXTERNAL_ENV" | ||
| ); | ||
| } else { | ||
| assert!( | ||
| !headers.contains_key("datadog-external-env"), | ||
| "datadog-external-env header should not be present when entity_id::DD_EXTERNAL_ENV is None" | ||
| ); | ||
| } |
Member
There was a problem hiding this comment.
Hmmm I'm not the biggest fan of these tests, since they change their behavior dynamically based on the exact execution environment of the test runner, which means that
a) Whatever I get running on my machine may be different -- so maybe it fails in CI but never for me
b) We don't get full test coverage -- only whatever codepaths end up being picked
Is it possible to maybe mock the results of those functions for our tests?
That is, what I would do if this were Ruby would be to zoom out and say "the behavior in my headers is -- if each of these entries is available, the headers contain what got returned, and if there was nothing than the headers don't exist"
Then I'd mock container-id: [dummy-container-id, none], entity-id: [dummy-entity-id, none], external-env: [dummy-external-env, none] and check that I get the correct behavior in each of the 6 cases.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Unifies the headers between common and profiling
Motivation
We had missed some when we implemented our own exporter, this makes sure the two implementations stay in sync.
Additional Notes
Anything else we should know when reviewing?
How to test the change?
Describe here in detail how the change can be validated.