Transforming spans prior to export

[spencerwilson]

👋

Problem

Some string-valued span attributes contain information that should not be logged. My Node.js application is exporting to Collector Contrib. There are two processors that stand out as relevant:

• redaction ([processor/redaction] Initial implementation opentelemetry-collector-contrib#6495)
• transform (related doc)

Both are works in progress, and as such neither is included in the current Collector Contrib, 0.46.0.

Working solution

I thought I'd try making a RedactionSpanProcessor, which is identical to SimpleSpanProcessor except

--- a	2022-03-03 11:16:29.000000000 -0800
+++ b	2022-03-03 11:16:43.000000000 -0800
@@ -58,7 +58,7 @@
 
     // prevent downstream exporter calls from generating spans
     context.with(suppressTracing(context.active()), () => {
-      this._exporter.export([span], result => {
+      this._exporter.export([redactSpan(span)], result => {
         if (result.code !== ExportResultCode.SUCCESS) {
           globalErrorHandler(
             result.error ??

I first attempt at redactSpan went like

function redactSpan(span: ReadableSpan): ReadableSpan {
  return {
    ...span,
    attributes: redactAttributes(span.attributes),
  };
}

but I guess the own && enumerable properties copied by the spread operator didn't suffice; these spans subsequently error:

[watch-server] TypeError: span.spanContext is not a function
[watch-server]     at ConsoleSpanExporter._exportInfo (/Users/ssw/code/api/node_modules/@opentelemetry/sdk-trace-base/build/src/export/ConsoleSpanExporter.js:47:27)
[watch-server]     at ConsoleSpanExporter._sendSpans (/Users/ssw/code/api/node_modules/@opentelemetry/sdk-trace-base/build/src/export/ConsoleSpanExporter.js:66:30)
[watch-server]     at ConsoleSpanExporter.export (/Users/ssw/code/api/node_modules/@opentelemetry/sdk-trace-base/build/src/export/ConsoleSpanExporter.js:32:21)
[watch-server]     at /Users/ssw/code/api/devBuild/RedactionSpanProcessor.js:60:34

The following works, though!

function redactSpan(span: ReadableSpan): ReadableSpan {
  const result = Object.create(span);
  result.attributes = redactAttributes(span.attributes);
  return result;
}

Questions

1. Is forking SimpleSpanProcessor the easiest way to add some custom "transform attributes prior to export" logic today?
2. Are there any concerns with my Object.create(span) technique for "cloning" + modifying a Span? Is there a more advisable/supported way to do this?

Thank you!

[vmarchaud]

Is forking SimpleSpanProcessor the easiest way to add some custom "transform attributes prior to export" logic today?

Yes, this would be the most pratical way to do this.

Are there any concerns with my Object.create(span) technique for "cloning" + modifying a Span? Is there a more advisable/supported way to do this?

You could also do the following: Object.assign(span, { attributes redactAttributes(span.attributes) }) which will avoid cloning the original span.

[spencerwilson]

Oh, I'd expected tsc to give a type error for that, considering that ReadableSpan.attributes is a readonly property. But today I learned:

type x = { readonly a: boolean };
type y = { a: boolean };

// This is not equivalent to `never`!
type n = typeof x & typeof y;

TypeScript Handbook mentions this:

TypeScript doesn’t factor in whether properties on two types are readonly when checking whether those types are compatible, so readonly properties can also change via aliasing.

Tricky!

[Flarna]

You could also create your own Span,Tracer and TracerProvider classes by extending the corresponding ones for sdk-trace-base which filters attributes before they are actually stored in the span.
This has the advantage that all export chains export the same span not only a specific exporter.

[spencerwilson]

thanks!

[bgentry]

The suggested approach to create my own Span, Tracer, and TracerProvider turns out to be fairly complicated and involves copying lots of code because there aren't good points of extension for this and because you need each of those classes to know how to use the other class at various places (particularly when constructing the custom Span class, which happens at the end of a long method in the Tracer).

I opted for the simpler approach from the OP with a custom RedactingSpanProcessor based on SimpleNodeProcessor. This is the most minimal implementation and seems much less likely to break and drift from upstream changes vs the other approach where a lot of code would get copied in.

const redactAttributes: string[] = ['graphql.execute.variables', 'graphql.resolver.args'];

class RedactingSpanProcessor extends SimpleSpanProcessor {
  onEnd(span: ReadableSpan): void {
    const attributes = span.attributes;

    for (const key of redactAttributes) {
      const value = attributes[key];
      if (value) {
        const redacted = redact(value);
        span.attributes[key] = redacted;
      }
    }

    // call underlying SimpleSpanProcessor:
    super.onEnd(span);
  }
}

Put whatever logic you want in your redact function to rewrite the underlying value.

[juicemia]

How do you register this? Would setting this array override the default span processor?

For example, if I set the processor like this:

  const sdk = new NodeSDK({
    serviceName,
    spanProcessors: [
      new CustomSpanProcessor(),
    ],
    instrumentations: [
          // stuff
    ],
  })

Does all the default processing behavior get replaced by whatever my single CustomSpanProcessor does?

If that's the case, then would it be possible to add the default processors back to the array like this?

  const sdk = new NodeSDK({
    serviceName,
    spanProcessors: [
      new CustomSpanProcessor(),
      new BatchSpanProcessor()
    ],
    instrumentations: [
          // stuff
    ],
  })

I don't want to lose the default behavior, I just want to add a little bit to what it already does.

[bgentry]

Our setup is roughly like this:

// For troubleshooting, set the log level to DiagLogLevel.DEBUG
  diag.setLogger(new DiagConsoleLogger(), DiagLogLevel.WARN);

  const resource = detectResourcesSync({
    detectors: [envDetectorSync, processDetectorSync, dockerCGroupV1Detector],
  });

  const httpInstrumentation = new HttpInstrumentation({});

  const traceExporter = new OTLPTraceExporter({
    headers: {},
    url: `http://${mustEnv('OTEL_COLLECTOR_HOST')}:${mustEnv('OTEL_COLLECTOR_PORT')}/v1/traces`,
  });

  const telemetryProvider = new NodeTracerProvider({ resource });

  const sdk = new NodeSDK({
    instrumentations: [httpInstrumentation],
    resource,
    spanProcessors: [new RedactingSpanProcessor(traceExporter)],
    traceExporter: traceExporter,
  });