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

Disconnected traces when using @opentelemetry/instrumentation-grpc with @openfeature/flagd-provider #3775

Closed
pichlermarc opened this issue May 3, 2023 · 6 comments · Fixed by #3804
Assignees
Labels
bug Something isn't working pkg:instrumentation-grpc priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect

Comments

@pichlermarc
Copy link
Member

What happened?

Steps to Reproduce

  • clone reproduction repository
  • npm install
  • npm run docker:start
    • this will spin up flagd, otelcol and jaeger
  • npm run start
  • Go to http://localhost:16686

Expected Result

The trace should start at the js-client service and continue to flagd, however, the trace is broken and two different trace-ids are used, and no grpc spans can be seen.

Actual Result

The OpenFeature FlagdProvider uses @grpc/grpc-js to communicate with flagd, so by using @opentelemetry/instrumentation-grpc, the trace should be connected, the same trace-id should be used across all serivces, and a span should be created for the service call.

OpenTelemetry Setup Code

// see reproduction repository

package.json

{
  "name": "example-flagd-otel",
  "private": true,
  "version": "0.38.0",
  "description": "Example of using flagd with otel tracing",
  "main": "index.js",
  "scripts": {
    "start": "node --require ./traces.js client.js",
    "docker:start": "cd ./docker && docker-compose down && docker-compose up",
    "docker:startd": "cd ./docker && docker-compose down && docker-compose up -d",
    "docker:stop": "cd ./docker && docker-compose down"
  },
  "dependencies": {
    "@openfeature/js-sdk": "^1.2.0",
    "@openfeature/flagd-provider": "^0.7.6",
    "@opentelemetry/api": "^1.4.1",
    "@opentelemetry/context-async-hooks": "1.12.0",
    "@opentelemetry/core": "1.12.0",
    "@opentelemetry/exporter-trace-otlp-proto": "0.38.0",
    "@opentelemetry/instrumentation": "0.38.0",
    "@opentelemetry/instrumentation-grpc": "0.38.0",
    "@opentelemetry/resources": "1.12.0",
    "@opentelemetry/sdk-trace-base": "1.12.0",
    "@opentelemetry/sdk-trace-node": "1.12.0",
    "@opentelemetry/semantic-conventions": "1.12.0"
  }
}

Relevant log output

No response

@pichlermarc pichlermarc added bug Something isn't working priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect pkg:instrumentation-grpc labels May 3, 2023
@pichlermarc pichlermarc self-assigned this May 3, 2023
@Kavindu-Dodan
Copy link

Thank you @pichlermarc for recreating and looking into the issue.

Two minor remarks

  • The currently released flagd version lacks grpc tracing and require a custom built. This is pending the release of feature [1]. However, observation of missing traces is true for source based deployment
  • flagd exporter configurations are documented for debugging reference [2]

[1] - open-feature/flagd#624
[2] - https://github.com/open-feature/flagd/blob/main/docs/configuration/flagd_telemetry.md#export-to-otel-collector

@Kavindu-Dodan
Copy link

Update - our most recent flagd version v0.5.3 [1] contains the built-in tracing setup.

Besides, I composed a minimal multi-language setup to recreate the problem [2] in which Go & Java work as expected.

[1] - https://github.com/open-feature/flagd/pkgs/container/flagd/90724874?tag=v0.5.3
[2] - https://github.com/Kavindu-Dodan/otel-samples-flagd/tree/main

@pichlermarc
Copy link
Member Author

pichlermarc commented May 5, 2023

Looks like this is because the client generated by the protobuf-ts tooling never uses makeGenericClientConstructor or makeClientConstructor, therefore, the service client is never instrumented. To fix this we'll need to instrument the Client from @grpc/grpc-js itself (https://github.com/grpc/grpc-node/blob/892f16e1752d892d2b5879ca48dab731e655a1a7/packages/grpc-js/src/client.ts).

@Kavindu-Dodan
Copy link

@pichlermarc do you have any new update on this issue :) ?

@pichlermarc
Copy link
Member Author

pichlermarc commented May 15, 2023

Hi, I'm still working on it but the fix is taking longer than initially expected.
We need to change some things in the instrumentation as it relies quite a bit on the "normal" way of using the library. I'm hoping to open up a draft PR today, though.

EDIT: A draft PR is open but I'll still need to clean it up and add a bunch of tests.

@Kavindu-Dodan
Copy link

Thank you for the update @pichlermarc

Let me know if you need anything from our end. Happy to help you with validations of the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:instrumentation-grpc priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect
Projects
None yet
2 participants