scwhittle commented on code in PR #39625:
URL: https://github.com/apache/beam/pull/39625#discussion_r3734689187
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingHandler.java:
##########
@@ -606,6 +623,16 @@ public synchronized void publishToDisk(
writeIfNotEmpty(generator, "work", DataflowWorkerLoggingMDC.getWorkId());
writeIfNotEmpty(generator, "logger", record.getLoggerName());
writeIfNotEmpty(generator, "exception",
formatException(record.getThrown()));
+
+ if (logOpenTelemetryTraceSpanIdAndSampled.get()) {
+ SpanContext spanContext = Span.current().getSpanContext();
+ if (spanContext.isValid() && spanContext.isSampled()) {
+ generator.writeStringField("trace", spanContext.getTraceId());
+ generator.writeStringField("spanId", spanContext.getSpanId());
+ generator.writeBooleanField("trace_sampled",
spanContext.isSampled());
Review Comment:
I meant to just omit
`"trace_sampled": false`
since that is probably common and we could just infer it was false.
This is a little different since we're also not logging the other fields.
Is there still value in adding the trace and spanId to the log if it is
not-sampled? If not, this seems better to avoid unnecessary writes/parsing.
but perhaps there could be some use for these fields, if they could correlate
to other logs even if it isn't sampled? That is indicated by
https://docs.cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry#FIELDS.trace_sampled
I think it would be better then to log trace/spanId if the context is valid.
And just conditionally log trace_sampled if it is valid and true.
Alternatively we could add another option controlling if we only log sampled
trace/spanids.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
