feat(oracledb): propagate context using V$SESSION.ACTION#3226
feat(oracledb): propagate context using V$SESSION.ACTION#3226pichlermarc merged 6 commits intoopen-telemetry:mainfrom
Conversation
|
cc @sudarshan12s @sharadraju (component owners) please review 🙂 |
|
Correct link for the "Spec", I think: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/db/oracledb.md#vsessionaction |
|
@sudarshan12s @sharadraju any chance you could take a look? |
| export function buildTraceparent( | ||
| spanContext: SpanContext | ||
| ): string | undefined { | ||
| return `00-${spanContext.traceId}-${spanContext.spanId}-0${Number( |
There was a problem hiding this comment.
traceFlags is already a number , we do not need to use Number right?
I think we can use below?
(spanContext.traceFlags ?? TraceFlags.NONE)
.toString(16)
.padStart(2, '0');
| * | ||
| * @default false | ||
| */ | ||
| traceContextPropagation?: boolean; |
There was a problem hiding this comment.
The database wire protocol latest version supports passing the traceContext from client to server. The DB server can then start a child span of the corresponding client span.
I have plans to add it in this module after few node-oracledb driver changes..
The ACTION field could be used by application and hence the native change of wire protocol would resolve that issue (also the limit of 64 bytes with ACTION field)
Hence can we rename this attribute to
propagateTraceContextToSessionAction , so that when a wire protocol change is added, we can use this flag, traceContextPropagation ?
Also can we add the same in README.md
| propagateTraceContextToSessionAction | boolean | true | If true, injects the W3C Trace Context into the Oracle V$SESSION.ACTION field. This allows the OpenTelemetry Collector to correlate application traces with database server spans. |
There was a problem hiding this comment.
Tedious uses enableTraceContextPropagation so to be honest I am not sure why didn't use the same one here. Spec doesn't specify it so we can use whatever. Would make sense for all DBs to use the same one tho. I guess oracledb only one who will use action for this so I'll use propagateTraceContextToSessionAction.
Sorry for delay in checking this. Thanks for the changes. |
sudarshan12s
left a comment
There was a problem hiding this comment.
LGTM
I see some lint issue, minor formatting fixes:
npx prettier --write \
packages/instrumentation-oracledb/src/OracleTelemetryTraceHandler.ts \
packages/instrumentation-oracledb/test/oracle.test.ts
pichlermarc
left a comment
There was a problem hiding this comment.
proxy-approving for component owners.
Adds context propagation via V$SESSION.ACTION. Spec. It seems that requestHook doesn't have access to the connection so I couldn't use that.