googleapis / nodejs-logging

Node.js client for Stackdriver Logging: Store, search, analyze, monitor, and alert on log data and events from Google Cloud Platform and Amazon Web Services (AWS).
https://cloud.google.com/logging/
Apache License 2.0
168 stars 99 forks source link

logging: OpenTelemetry spans are ignored as parents for LogRecords #1490

Closed cindy-peng closed 1 month ago

cindy-peng commented 5 months ago

Tracking the same issue as 9302 to propagate correct TraceId and SpanId for logging and tracing correlation.

Client

nodejs-logging

Context

In nodejs-logging libraries, the current implementation will always use a remote trace ID and span ID parsed from HTTP headers that originate from outside the application: https://github.com/googleapis/nodejs-logging/blob/53c5c248b05dd987ee8675d1d250261698ba9ba3/src/utils/context.ts#L95

As a result, logs are associated with the parent span rather than the span of interest.

Expected behavior

If the user is using opentelemetry instrumentation, we want to use the traceID and spanID of the span that OpenTelemetry created. This way, when a user looks for logs associated with that span, the logs produced by this logging library will appear.

cindy-peng commented 5 months ago

To fix this issue in nodejs logging repo, @dashpole suggested importing OpenTelemetry Api and using nodejs spanContext to retrieve current traceId and spanId:https://github.com/googleapis/google-cloud-go/issues/9302#issuecomment-1930391821

@aabmass David mentioned that you may help review the fixes for nodejs and java. I added the following logic to retrieve spanContext locally and it seemed working fine locally:

import { trace, context, isSpanContextValid} from '@opentelemetry/api';
import { isTracingSuppressed } from '@opentelemetry/core';

   const activeContext = context.active();
    const spanContext = trace.getSpanContext(activeContext);

    if (spanContext != undefined && isSpanContextValid(spanContext) && !isTracingSuppressed(activeContext)) {
      const otelSpanContext = {
        trace: spanContext?.traceId,
        spanId: spanContext?.spanId,
        traceSampled: spanContext?.traceFlags === 1,
      };
    }

I am wondering if you can help with a couple of questions:

  1. is context.active() the correct way to get the current active context and we can then use it to retrieve spanContext? It's mentioned that context.active() will work only with context Manager configured, otherwise it will always return ROOT_CONTEXT: https://opentelemetry.io/docs/languages/js/context/#active-context Do we need to configure context Manger in that case?

  2. Do you think we need to validate !isTracingSuppressed(activeContext) as well? Looks like this is introduced from opentelemetry-core

    Thank you!