Backend dependencies granularity for NoSQL and Messaging#2104
Merged
stevejgordon merged 11 commits intomainfrom Jul 12, 2023
Merged
Backend dependencies granularity for NoSQL and Messaging#2104stevejgordon merged 11 commits intomainfrom
stevejgordon merged 11 commits intomainfrom
Conversation
a2bb1b8 to
efbdf67
Compare
Contributor
Author
|
@Mpdreamz This is ready for review but is blocked as I do not currently have Azure access to run the updated Azure tests. Once these are enabled again in CI, we can run them there, or if one of us has access to run them locally, that should be sufficient for now. |
BREAKING: This removes the setting of context.db.instance and context.db.statement to align with the spec. Adds span assertions to validate specification conditions are met for spans.
- We now attach HTTP info to the context for the main span. BREAKING: - The action has been changed to 'request' to align with the spec. - The db.instance is no longer set. The spec recommends that this can be determined on cloud using `x-found-handling-cluster` but we don't have a way to access the headers from the client diagnostics (v7). Notes: `db.statement` impl has not been changed to align with the spec. That will be future work.
Includes a rename of `DisabledOnWindowsDockerFact` to include CI in the name so it is more explicit. These tests runs correctly on Windows when Docker is available.
b8eb193 to
ef4052c
Compare
Mpdreamz
approved these changes
Jul 11, 2023
Member
Mpdreamz
left a comment
There was a problem hiding this comment.
Small note other than that LGTM
b6afc27 to
e08d730
Compare
This was referenced Mar 27, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes #1754
This PR aligns more closely with the specification.
BREAKING:
context.db.instanceandcontext.db.statementto align with the spec.db.instanceis no longer set. The spec recommends that this be determined on Elastic Cloud usingx-found-handling-cluster, but we don't have a way to access the headers from the client diagnostics (v7).Notes:
For Elasticsearch, the
db.statementimplementation has not been changed to align with the spec as that requires more extensive changes and will be addressed as part of #1785.Includes