-
Notifications
You must be signed in to change notification settings - Fork 375
[db] Add docs for context info propagation #2236
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
Changes from 4 commits
1529979
ab6e386
08b02a8
2c26648
0ccfda5
a6e65f4
886aa37
f633fa2
5b835f8
e021083
1bcbf0d
e6803f5
f986ee2
8794c1d
ec00e3c
fa6cf6d
2c0e01a
430957b
7f05f5e
5534272
9876582
89a4baf
0e7ff53
8effebc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # Use this changelog template to create an entry for release notes. | ||
| # | ||
| # If your change doesn't affect end users you should instead start | ||
| # your pull request title with [chore] or use the "Skip Changelog" label. | ||
|
|
||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: enhancement | ||
|
|
||
| # The name of the area of concern in the attributes-registry, (e.g. http, cloud, db) | ||
| component: db | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: Add doc for context information propagation | ||
|
|
||
| # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. | ||
| # The values here must be integers. | ||
| issues: [2162] | ||
|
|
||
| # (Optional) One or more lines of additional information to render under the primary note. | ||
| # These lines will be padded with 2 spaces and then inserted directly into the document. | ||
| # Use pipe (|) for multiline entries. | ||
| subtext: |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -13,6 +13,7 @@ linkTitle: Spans | |||
| - [Notes and well-known identifiers for `db.system.name`](#notes-and-well-known-identifiers-for-dbsystemname) | ||||
| - [Sanitization of `db.query.text`](#sanitization-of-dbquerytext) | ||||
| - [Generating a summary of the query](#generating-a-summary-of-the-query) | ||||
| - [Propagating context to databases](#propagating-context-to-databases) | ||||
| - [Semantic conventions for specific database technologies](#semantic-conventions-for-specific-database-technologies) | ||||
|
|
||||
| <!-- tocstop --> | ||||
|
|
@@ -478,6 +479,41 @@ Semantic conventions for individual database systems or specialized instrumentat | |||
| MAY specify a different `db.query.summary` format as long as produced summary remains | ||||
| relatively short and its cardinality remains low comparing to the `db.query.text`. | ||||
|
|
||||
| ## Propagating context to databases | ||||
|
|
||||
| **Status**: [Development][DocumentStatus] | ||||
|
|
||||
| Instrumentations SHOULD propagate the context information to databases. | ||||
|
pellared marked this conversation as resolved.
Outdated
|
||||
|
|
||||
| | Attribute | Type | Description | Require level | Stability | | ||||
|
pellared marked this conversation as resolved.
Outdated
|
||||
| |----------------|--------|---------------------------------------|-------------------|----------------------------------------------------------------| | ||||
| | `service.name` | string | Logical name of the service [1] | `Required` |  | | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this needed?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is because of the limitation of actual implementation. When propagating Some questions here:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @XSAM, it is reasonable to me to have this attribute when traceid cannot be injected due to performance reasons.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotcha. In such case
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've updated this PR and still keep the sheet to represent the minimal data that should be propagated. Please let me know if you feel it's unnecessary.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We somehow need to propagate this information. I was thinking of reusing some existing propagator, instead of creating a new one. The alternative would be probably creating a new "service name propagator". However, it feels that a baggage propagator can be used for this. From https://www.w3.org/TR/baggage/#example-use-case:
I just noticed that here is a nice doc for context propagation for AWS: https://github.com/open-telemetry/semantic-conventions/blob/45e91aedff7d347955ba09269b871f8fd9049ce3/docs/non-normative/compatibility/aws.md#context-propagation
Example:
I think that we just need to write it down as I do not think we have any semantic conventions for baggage key-values so far. CC @reyang, @dyladan, @yurishkuro, @arminru
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pellared Do you recommend using the format
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest exploring
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pellared Updated the examples accordingly. |
||||
| | `traceparent` | string | The trace context of current span [2] | `Recommended` [3] |  | | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Open question, not a blocker for the PR, can be considered separately. |
||||
|
|
||||
| **[1] `service.name`:** MUST be the same for all instances of horizontally scaled services. If the value was not specified, SDKs MUST fall back to `unknown_service:` concatenated with [process.executable.name](https://opentelemetry.io/docs/specs/semconv/attributes-registry/process/), e.g. `unknown_service:bash`. If `process.executable.name` is not available, the value MUST be set to `unknown_service`. | ||||
|
|
||||
| **[2] `traceparent`:** MUST be in the [text format](https://www.w3.org/TR/trace-context/#traceparent-header). | ||||
|
|
||||
| **[3] `traceparent`:** `tracparent` have extremely high-cardinality. It's RECOMMENDED to propagate this info if the high-cardinality is safe for the behind databases | ||||
|
sincejune marked this conversation as resolved.
Outdated
|
||||
|
|
||||
| Instrumentations SHOULD propagate the context information to the SQL queries following [sqlcommenter](https://google.github.io/sqlcommenter/spec/). | ||||
|
|
||||
| **Examples:** | ||||
|
|
||||
| - Query with `service.name`: | ||||
|
|
||||
| ```sql | ||||
| SELECT * FROM songs /* service.name=music-player:play */ | ||||
| ``` | ||||
|
|
||||
| - Query with `service.name` and `traceparent` | ||||
|
|
||||
| ```sql | ||||
| SELECT * FROM songs /* service.name=music-player:play, traceparent=00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01 */ | ||||
|
pellared marked this conversation as resolved.
Outdated
|
||||
| ``` | ||||
|
|
||||
| Instrumentation SHOULD propagate `traceparent` as part of the [sqlcommenter](https://google.github.io/sqlcommenter/spec/) if high-cardinality of `traceparent` is safe to the specific databases. | ||||
|
|
||||
| ## Semantic conventions for specific database technologies | ||||
|
|
||||
| More specific Semantic Conventions are defined for the following database technologies: | ||||
|
|
||||
Uh oh!
There was an error while loading. Please reload this page.