Add telemetry completeness tests #3650
Merged
Conversation
This file contains 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
cbeauchesne
requested changes
Dec 10, 2024
bm1549
commented
Dec 10, 2024
@cbeauchesne I've fixed the CI and all failures are now unrelated to the new test. Please re-review 🙏 |
cbeauchesne
requested changes
Dec 11, 2024
|
Could you also update your branch? it'll fix php errors |
mabdinur
reviewed
Dec 11, 2024
robertomonteromiguel
previously requested changes
Dec 12, 2024
bm1549
commented
Dec 12, 2024
| elif isinstance(obj, list): | ||
| result.extend(obj) | ||
| else: | ||
| logger.error(f"Unexpected type in concat: {type(obj).__name__}") |
There was a problem hiding this comment.
If this occurs, I expect the test to fail due to missing config keys, so not throwing here
bm1549
force-pushed
the
bm1549/config-telemetry
branch
from
5d696a3 to
93a21f1
Compare
bm1549
commented
Dec 12, 2024
| # If this script deletes the JSON files in system-tests, it's likely this is no longer valid | ||
| SUBDIR="trace/apps/tracer-telemetry-intake/telemetry-payload/static/" | ||
| # The "main" branch in dd-go | ||
| BRANCH="bm1549/more-missing-configs" # This should ALWAYS be "prod" in master. Please revert changes before merging |
There was a problem hiding this comment.
Pending (but not blocked by) https://github.com/DataDog/dd-go/pull/161868
Suggested change
| BRANCH="bm1549/more-missing-configs" # This should ALWAYS be "prod" in master. Please revert changes before merging | |
| BRANCH="prod" |
cbeauchesne
approved these changes
Dec 13, 2024
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.
Motivation
Adds a new test for telemetry completeness, ensuring what is specified in dd-go is present in our individual libraries
With the current implementation, this tests for telemetry of an empty configuration which may be lossy in some languages
These tests somewhat eliminate the need for equivalent tests in dd-trace-js and tests in dd-trace-dotnet (missing ref)
Note for reviewer: I'm a bit suspicious about the correctness of this line, but it seems to better match what is on this notebook
Changes
Adds a test across languages
Workflow
codeownersfile quickly.🚀 Once your PR is reviewed, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
[<language>], double-check that only<language>is impacted by the changebuild-XXX-imagelabel is present