Instrumenting SDKs with open telemetry #69
Conversation
JakeGinnivan
changed the title
JakeGinnivan
force-pushed
the
feature/open-telemetry
branch
from
d77f7fd
to
be98aae
Compare
| @@ -41,29 +45,18 @@ export function createBrowserClient({ | |||
|
|
|||
| environmentApiKey: string | |||
| }): BrowserClient { | |||
| const initialPromise = new PromiseCompletionSource<boolean>() | |||
| const tracer = getTracer() | |||
There was a problem hiding this comment.
This is probably the riskiest change in the PR.
I rewrite the retry logic and removed a bunch of duplication in this function based on the traces not quite looking right.
| import { http, passthrough } from 'msw' | ||
|
|
||
| export const openTelemetryTracePassthrough = http.post( | ||
| 'http://localhost:4318/v1/traces', |
There was a problem hiding this comment.
Is this meant to be hardcoded?
There was a problem hiding this comment.
Yeah, it's just for unit tests if you configure open telemetry locally using Jaeger it will pass the trace calls through.
This wouldn't work if you wanted to use honeycomb or app insights for unit test telemetry though
| externalStateStore, | ||
| ) | ||
| if (process.env.FEATUREBOARD_SDK_DEBUG) { | ||
| stateStore = new DebugFeatureStateStore(stateStore) |
There was a problem hiding this comment.
I used a decorator pattern to reduce overhead when not debugging
libs/node-sdk/project.json
Outdated
| @@ -21,6 +21,7 @@ | |||
| "executor": "nx:run-commands", | |||
| "options": { | |||
| "commands": [ | |||
| "echo \"export const version = '$(jq -r '.version' package.json)';\" > src/version.ts", | |||
There was a problem hiding this comment.
Isn't this going to cause issues when you publish as there will be non committed changes in git.
There was a problem hiding this comment.
I've run the command before we do the commit, so the version number update should get checked in.
| return initialisedState.initialisedPromise.completed | ||
| const err = resolveError(error) | ||
|
|
||
| initializeSpan.setStatus({ |
There was a problem hiding this comment.
Already set inside retry when fail, do we need to set it on this span as well?
…EBUG env variable
Tests can export to OTEL if configured