-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update 128 bit logging enabled tests and disable stable config tests #4251
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some questions and minor suggestions. Feel free to merge the PR when CI is green.
tests/test_config_consistency.py
Outdated
# dd-trace-java stores injected trace information under the "mdc" key | ||
if context.library.library == "java": | ||
log_msg = log_msg.get("mdc") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this logic to parse_log_injection_message
so we can use use it in tests
'*': incomplete_test_app (weblog endpoint not implemented) | ||
'spring-boot-3-native': missing_feature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we planning to enable the 128bit logging tests in this PR?
'*': incomplete_test_app (weblog endpoint not implemented) | |
'spring-boot-3-native': missing_feature | |
'*': incomplete_test_app (weblog endpoint not implemented) | |
'spring-boot-3-native': v1.48.0.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh we could do it like this, but I was going open a new PR to enable all tests together anyways. This was just to update the manifests to help Java CI pass
Motivation
Update 128bit logging enabled tests to consider the Java edge case. Update and disable stable config Java tests to allow CI to pass on the following dd-trace-java PR that updates the default value of
DD_LOGS_INJECTION
. Stable Config tests will be enabled in a future PR following the merge of the above dd-trace-java PR.Changes
Workflow
codeowners
file 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-image
label is present