open-telemetry / opentelemetry-js

OpenTelemetry JavaScript Client
https://opentelemetry.io
Apache License 2.0
2.64k stars 763 forks source link

B3 propagator searching for headers with lowercase #3150

Open uralsemih opened 2 years ago

uralsemih commented 2 years ago

What happened?

Hello,

I have a Java service(producer) and Nodejs service(consumer) where they are connecting to Kafka. Java service sends a JSON object to Kafka and Node.js service consumes it and saves it to the database.

I am using b3-propagators multi and when I made a comparison to traces between services I am aware that the Kafka one has a different one where it breaks the traces.If you check the consumer message and java the traces are same which is 2721916b8dbb8e1d58d6d269a10b056d where Kafka one 2273c5c3fe7cd6edc27f3c3c8366ab7e..

Seems the root cause of this issue is case sensitivity where b3 propagator searching for headers with lowercase which is not according to spec.

Can someone have a look at this, please?

opentelemetry/propagator-b3

/** B3 single-header key */
export const B3_CONTEXT_HEADER = 'b3';

/* b3 multi-header keys */
export const X_B3_TRACE_ID = 'x-b3-traceid';

opentelemetry/opentelemetry-java

@Immutable
public final class B3Propagator implements TextMapPropagator {
  static final String TRACE_ID_HEADER = "X-B3-TraceId";

OpenTelemetry Setup Code

const { getNodeAutoInstrumentations } = require("@opentelemetry/auto-instrumentations-node");
const { registerInstrumentations } = require('@opentelemetry/instrumentation');
const opentelemetry = require("@opentelemetry/sdk-node");
const api = require('@opentelemetry/api');
const { CompositePropagator } = require('@opentelemetry/core');
const { Resource } = require('@opentelemetry/resources');
const { SemanticResourceAttributes } = require('@opentelemetry/semantic-conventions');
const { B3Propagator, B3InjectEncoding } = require('@opentelemetry/propagator-b3');
const {ConsoleSpanExporter} = require('@opentelemetry/sdk-trace-base');
// const { OTLPTraceExporter } = require('@opentelemetry/exporter-trace-otlp-grpc');
const { ExpressInstrumentation } = require('@opentelemetry/instrumentation-express');
const { KafkaJsInstrumentation } = require('opentelemetry-instrumentation-kafkajs');

const serviceName = 'product-service'
// const traceExporter = new OTLPTraceExporter({
//   url: process.env.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT || 'http://otel-collector:4317/'
// });

api.propagation.setGlobalPropagator(
    new CompositePropagator({
      propagators: [
        new B3Propagator({ injectEncoding: B3InjectEncoding.MULTI_HEADER }),
      ],
    })
);

const sdk = new opentelemetry.NodeSDK({
  resource: new Resource({
    [SemanticResourceAttributes.SERVICE_NAME]: serviceName,
  }),
  traceExporter: new ConsoleSpanExporter(),
  instrumentations: [getNodeAutoInstrumentations()]
});

registerInstrumentations({
    instrumentations: [
      new ExpressInstrumentation(),
      new KafkaJsInstrumentation()
    ],
});

sdk
  .start()
....

package.json

"dependencies": {
    "@opentelemetry/api": "^1.1.0",
    "@opentelemetry/auto-instrumentations-node": "^0.31.0",
    "@opentelemetry/exporter-trace-otlp-grpc": "^0.30.0",
    "@opentelemetry/instrumentation-express": "^0.30.0",
    "@opentelemetry/propagator-b3": "^1.4.0",
    "@opentelemetry/resources": "^1.4.0",
    "@opentelemetry/sdk-node": "^0.30.0",
    "@opentelemetry/semantic-conventions": "^1.4.0",
    "axios": "^0.27.2",
    "express": "^4.18.1",
    "kafkajs": "^2.1.0",
    "mongoose": "^6.4.6",
    "opentelemetry-instrumentation-kafkajs": "^0.29.0",
    "prom-client": "^14.0.1"
  }

Relevant log output

Node.js(consumer)
{
  partition: 0,
  offset: '0',
  value: '{"title":"shoes","desc":"shoes","img":"test4.svg","categories":["shoes","man"],"size":"M","price":"50"}',
  headers: {
    'X-B3-TraceId': '2721916b8dbb8e1d58d6d269a10b056d',
    'X-B3-SpanId': '4e0c3513fa73ac8d',
    'X-B3-Sampled': '1',
    traceparent: '00-2721916b8dbb8e1d58d6d269a10b056d-4e0c3513fa73ac8d-01',
    __TypeId__: 'com.basketservice.model.Products'
  }
}
{
  traceId: '2273c5c3fe7cd6edc27f3c3c8366ab7e',
  parentId: undefined,
  name: 'productCreated',
  id: '69392d37aade564a',
  kind: 4,
  timestamp: 1659635058364999,
  duration: 36058,
  attributes: {
    'messaging.system': 'kafka',
    'messaging.destination': 'productCreated',
    'messaging.destination_kind': 'topic',
    'messaging.operation': 'process'
  },
  status: { code: 0 },
  events: [],
  links: []
}
{
  traceId: '2273c5c3fe7cd6edc27f3c3c8366ab7e',
  parentId: '69392d37aade564a',
  name: 'mongodb.insert',
  id: '9ebe87bfc3bb5fbc',
  kind: 2,
  timestamp: 1659635058382611,
  duration: 16705,
  attributes: {
    'db.system': 'mongodb',
    'db.name': 'products',
    'db.mongodb.collection': 'products',
    'net.host.name': '192.168.64.4',
    'net.host.port': '27017',
    'db.statement': '{"title":"?","desc":"?","img":"?","categories":"?","size":"?","price":"?","_id":"?","createdAt":"?","updatedAt":"?","__v":"?"}'
  },
  status: { code: 0 },
  events: [],
  links: []
}

Java(producer)

2022-08-04 17:44:17.314 trace_id=2721916b8dbb8e1d58d6d269a10b056d span_id=dc350ce3af0132d2 trace_flags=01  INFO 7 --- [ctor-http-nio-3] c.a.b.controller.BasketController        : PRODUCT ADDED Products(title=shoes, desc=shoes, img=test4.svg, categories=[shoes, man], size=M, price=50)
2022-08-04 17:44:17.438 trace_id=2721916b8dbb8e1d58d6d269a10b056d span_id=dc350ce3af0132d2 trace_flags=01  INFO 7 --- [ctor-http-nio-3] o.a.k.clients.producer.ProducerConfig    : ProducerConfig values:
vmarchaud commented 2 years ago

Node should automatically lower case header (https://nodejs.org/dist/latest-v16.x/docs/api/http.html#messageheaders) thats why we use lower case. The kafka instrumentation should instead lower case incoming headers i think

uralsemih commented 2 years ago

To be honest, it is hard to define for me which is a better approach but when I checked b3-propagation seems they're expecting uppercase rather than lowercase.

legendecas commented 2 years ago

HTTP header keys are case insensitive (RFC 2616 4.2 Message Headers), that's why Node.js is converting all incoming HTTP header keys to lower case.

For B3 propagation in other transports, I find that there is a note about the preference of case sentitivity in their document:

https://github.com/openzipkin/b3-propagation#multiple-headers Note: Http headers are case-insensitive, but sometimes this encoding is used for other transports. When encoding in case-sensitive transports, prefer lowercase keys or the single header encoding which is explicitly lowercase.

If I read that correctly, lowercase keys are still the preferred way.

Flarna commented 2 years ago

Seems like this is inconsistent across techs. Java doesn't use lowercase keys..

I think the OTel spec should clarify here to avoid interoperability issues like this.

dyladan commented 2 years ago

There is also a difference between "prefer lowercase keys" and "ignore non-lowercase keys". It could be interpreted to prefer the lowercase version if there is a collision.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] commented 1 year ago

This issue was closed because it has been stale for 14 days with no activity.

chenlujjj commented 1 year ago

Encountered a similar issue, do we have any updates later?

seemk commented 11 months ago

Can confirm this causes split traces. Could this get a never stale tag?

helloitslore commented 2 months ago

Hi everyone, I don't know if this could be helpful for someone, but I fixed it like this: Basically, I created a wrapper around B3Propagator and then I simply lowercased the carrier's keys.

import { B3Propagator } from '@opentelemetry/propagator-b3';
import { Context, TextMapGetter } from '@opentelemetry/api';

export class LowerCaseB3Propagator extends B3Propagator {
  public extract(context: Context, carrier: unknown, getter: TextMapGetter): Context {
    return super.extract(context, this.toLowerCase(carrier), getter);
  }

  private toLowerCase(carrier: any): any {
    if (carrier === undefined || carrier === null) return carrier;

    return Object.keys(carrier).reduce((result: any, field: string) => {
      const value = carrier[field];
      if (value !== undefined) {
        result[field.toLowerCase()] = value;
      }
      return result;
    }, {});
  }
}