CASSJAVA-63: Proposal for Native OpenTelemetry Tracing Support#1994
CASSJAVA-63: Proposal for Native OpenTelemetry Tracing Support#1994SiyaoIsHiding wants to merge 6 commits intoapache:4.xfrom
Conversation
proposals/open-telemetry/tracing.md
Outdated
There was a problem hiding this comment.
You can also mention that it will be officially supported and not vulnerable to internal API changes.
|
|
||
| **[2]:** Despite not being required, this implementation sets the `db.operation` attribute even if `db.statement` is included. | ||
|
|
||
| **[3]:** The statement value is the query string and does not include any query values. As an example, having a query that as the string `SELECT * FROM table WHERE x = ?` with `x` parameter of `123`, the attribute value of `db.statement` will be `SELECT * FROM table WHERE x = ?` and not `SELECT * FROM table WHERE x = 123`. |
There was a problem hiding this comment.
In extreme cases it may be beneficial to provide also set of bind parameters. For example poorly performing CQL query due to large partition can be discovered when seeing bound parameters. I do not say this feature is strictly needed, but maybe nice to have.
proposals/open-telemetry/tracing.md
Outdated
There was a problem hiding this comment.
If the intention is for driver core module not to declare dependency on OpenTelemetry, this will not be possible. I think we should declare OPTL integration via specific RequestTracker implementation. When creating a CqlSession developer will use withRequestTracker() method to pass new OpenTelemetryRequestTracker(config). This way core module will not need to declare API dependency on OpenTelemetry.
There was a problem hiding this comment.
Thank you! Good idea!
In that way, will it still support configuration via application.yml?
proposals/open-telemetry/tracing.md
Outdated
There was a problem hiding this comment.
I think RequestTracker is missing close() method to release resources (thread pools, channels etc.).
proposals/open-telemetry/tracing.md
Outdated
There was a problem hiding this comment.
Where are we going to pass retry details, speculative execution and consistency level?
There was a problem hiding this comment.
Thanks for your review! I updated the proposal. Retries and speculative executions details, e.g. db.query.text and server.address will be in the attributes Node Request type of spans. But we can never know which Node Request is a retry or a speculative execution. The current RequestTracker interface does not expose such information.
No description provided.