ExpressJS instrumentation doesn't play nice with Sampler

[hermanbanken]

I want to exclude certain routes from the ExpressJS instrumentation. However, this is made extra hard by the updateName usage in the instrumentation. If I look at the spans in our visualizer (Cloud Trace) the name of the span I want to exclude is "GET /health" (the Kubernetes health check), however, in the sampler the name of the span is still the name that the HTTP library instrumentation gives the span.

I'd like to see some possibility to exclude certain routes from being traced completely, however this does not seem possible now.

updateName culprit:

opentelemetry-js-contrib/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts

Lines 205 to 207 in 5fb3313

	rpcMetadata.span.updateName(
	`${req.method} ${route.length > 0 ? route : '/'}`
	);

What version of OpenTelemetry are you using?

0.24 of core and contrib packages.

What version of Node are you using?

Node 14

What did you do?

export class FilterSampler implements Sampler {
  constructor(private parent: Sampler, private filter: (spanName: string) => boolean) {}
  public shouldSample(ctx: Context, tid: string, name: string, kind: SpanKind, attr: SpanAttributes, links: Link[]) {
    if (!this.filter(name)) {
      process.stderr.write(`Excluding span with name "${name}"\n`); // debug
      return { decision: SamplingDecision.NOT_RECORD };
    }
    process.stderr.write(`Sampling span with name "${name}"\n`); // debug
    return this.parent.shouldSample(ctx, tid, name, kind, attr, links);
  }
  public toString(): string {
    return `FilterSampler(${this.parent.toString()})`;
  }
}

const filterFn = (spanName: string) => spanName.indexOf("GET /health") < 0 && spanName.indexOf("GET /") < 0;
const sampler = new FilterSampler(new AlwaysOnSampler(), filterFn);
const provider = new NodeTracerProvider({ sampler });
provider.register();
trace.setGlobalTracerProvider(provider);
registerInstrumentations({
  instrumentations: [
    new HttpInstrumentation(),
    new ExpressInstrumentation(),
  ],
});

What did you expect to see?

Excluding span with name "GET /health"
Sampling span with name "middleware - query"
Sampling span with name "middleware - expressInit"
Sampling span with name "request handler - /health"

What did you see instead?

Sampling span with name "HTTP GET"
Sampling span with name "middleware - query"
Sampling span with name "middleware - expressInit"
Sampling span with name "request handler - /health"

Additional context

[Flarna]

If you want to filter the whole trace it has to start at http instrumentation - express comes later and at that time the http span has been started already.
Your sampler can check on server attributes like HTTP_ROUTE, HTTP_METHOD and maybe also SpanKind.

[hermanbanken]

I've now adapted the filter function to look at attributes too:

const filterFn = (spanName: string, attr: SpanAttributes) => {
  // http.route is only set on ExpressJS spans and Http server-side spans.
  if (attr["express.type"]) {
    return true; // allow ExpressJS plugin spans which have http.route too.
  }
  // Remove Http plugin (parent) spans for health checks
  const route = (attr["http.route"] as string | undefined);
  return route !== "/health" && route !== "/";
};

which fixes the issue.

However, I think this could use some examples in the docs! This is far from trivial to figure out for new developers.