Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to supply Open Telemetry traceparent with request #394

Open
DylanRJohnston opened this issue Feb 24, 2025 · 2 comments
Open

Ability to supply Open Telemetry traceparent with request #394

DylanRJohnston opened this issue Feb 24, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@DylanRJohnston
Copy link

Use case

Being able to associate the traces in system.opentelemetry_span_log with a client query. As documented here

Describe the solution you'd like

Clickhouse documentation states that

ClickHouse accepts trace context HTTP headers, as described by the W3C recommendation. It also accepts trace context over a native protocol that is used for communication between ClickHouse servers or between the client and server.

However there doesn't appear to be any way to set these given the current client interface. You can set the request headers once during createClient but for the headers to be useful they need to be settable for each query, as each query will have a different traceparent.

const result = await client.query({ query, query_params, headers: { traceparent }})
@DylanRJohnston DylanRJohnston added the enhancement New feature or request label Feb 24, 2025
@DylanRJohnston
Copy link
Author

DylanRJohnston commented Feb 25, 2025

I've temporarily found a work around for this using some gross Proxy magic and intercepting the serialisation of the header with Symbol.toPrimitive and then setting the module global currentSpan just before calling client.query. This is very fragile though as it relies upon the header being serialised without yielding back to the event loop potentially causing currentSpan to be set to a different value. While this is currently true, it is certainly not an invariant guaranteed by the API and could break at any time.

const client = createClient({
      url: process.env?.["CLICKHOUSE_URL"],
      username: process.env?.["CLICKHOUSE_USERNAME"],
      password: process.env?.["CLICKHOUSE_PASSWORD"],
      http_headers: {
        traceparent: new Proxy({}, {
          get: (target, prop) => {
            switch (prop) {
              case Symbol.toPrimitive:
                return () =>
                  `00-${currentSpan!.traceId}-${currentSpan!.spanId}-0${Number(
                    currentSpan!.sampled ? TraceFlags.SAMPLED : TraceFlags.NONE
                  ).toString(16)}`;
              default:
                return Reflect.get(target, prop);
            }
          },
        }) as string,
      },
    });

@DylanRJohnston
Copy link
Author

Following up from the above hack with Symbol.toPromitive, after further testing it is not at all reliable, I guess we do yield back to the event loop in between setting the global currentSpan and the header being serialised. I've opened a PR #395, that adds the ability to specify the headers when querying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant