scwhittle commented on code in PR #39625:
URL: https://github.com/apache/beam/pull/39625#discussion_r3720534873
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingHandler.java:
##########
@@ -385,7 +394,14 @@ LogEntry constructDirectLogEntry(
LogEntry.newBuilder(Payload.JsonPayload.of(payloadBuilder.build()))
.setTimestamp(Instant.ofEpochMilli(record.getMillis()))
.setSeverity(severityFor(record.getLevel()));
-
+ SpanContext spanContext = Span.current().getSpanContext();
Review Comment:
maybe better to just get the current span context if
logOpenTelemetryTraceSpanIdAndSampled is enabled in case there is some overhead
like a thread-local
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingHandler.java:
##########
@@ -606,6 +622,13 @@ public synchronized void publishToDisk(
writeIfNotEmpty(generator, "work", DataflowWorkerLoggingMDC.getWorkId());
writeIfNotEmpty(generator, "logger", record.getLoggerName());
writeIfNotEmpty(generator, "exception",
formatException(record.getThrown()));
+ SpanContext spanContext = Span.current().getSpanContext();
+ if (logOpenTelemetryTraceSpanIdAndSampled.get() &&
spanContext.isValid()) {
Review Comment:
ditto
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingHandler.java:
##########
@@ -344,6 +352,7 @@ LogEntry constructDirectLogEntry(
@Nullable DataflowExecutionState executionState,
ImmutableMap<String, String> defaultResourceLabels) {
Struct.Builder payloadBuilder = Struct.newBuilder();
+ //
Review Comment:
rm
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingHandler.java:
##########
@@ -606,6 +622,13 @@ public synchronized void publishToDisk(
writeIfNotEmpty(generator, "work", DataflowWorkerLoggingMDC.getWorkId());
writeIfNotEmpty(generator, "logger", record.getLoggerName());
writeIfNotEmpty(generator, "exception",
formatException(record.getThrown()));
+ SpanContext spanContext = Span.current().getSpanContext();
+ if (logOpenTelemetryTraceSpanIdAndSampled.get() &&
spanContext.isValid()) {
+ generator.writeStringField("trace", spanContext.getTraceId());
+ generator.writeStringField("spanId", spanContext.getSpanId());
+ generator.writeBooleanField("traceSampled", spanContext.isSampled());
Review Comment:
would it be better to reduce overhead by just writing this if it is true and
omitting it if false?
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingHandler.java:
##########
@@ -606,6 +622,13 @@ public synchronized void publishToDisk(
writeIfNotEmpty(generator, "work", DataflowWorkerLoggingMDC.getWorkId());
writeIfNotEmpty(generator, "logger", record.getLoggerName());
writeIfNotEmpty(generator, "exception",
formatException(record.getThrown()));
+ SpanContext spanContext = Span.current().getSpanContext();
+ if (logOpenTelemetryTraceSpanIdAndSampled.get() &&
spanContext.isValid()) {
+ generator.writeStringField("trace", spanContext.getTraceId());
Review Comment:
does isValid imply that the trace id and span id are not empty? otherwise
use writeIfNotEmpty
--
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]
