-
Notifications
You must be signed in to change notification settings - Fork 83
fix: propagate user hash to startup spans #683
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
Changes from 2 commits
90a4577
71a88de
817aebd
74baec0
ab02db0
0e14abf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "hevy-mcp": patch | ||
| --- | ||
|
|
||
| Use the OpenTelemetry `user.hash` semantic convention and propagate the user hash to every recorded span. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ import { debugLog } from "./debug.js"; | |||||||||||||||||
| import type { HevyClientOptions } from "./hevyClientKubb.js"; | ||||||||||||||||||
| import { apiCalls, apiDuration } from "./metrics.js"; | ||||||||||||||||||
| import { createSafeErrorDiagnostic } from "./safe-error-diagnostic.js"; | ||||||||||||||||||
| import { getCurrentUserId, tracer } from "./telemetry.js"; | ||||||||||||||||||
| import { getCurrentUserHash, tracer } from "./telemetry.js"; | ||||||||||||||||||
|
|
||||||||||||||||||
| /** Node-only adapter; the Worker graph never imports telemetry or metrics. */ | ||||||||||||||||||
| export function createNodeHevyClientOptions(): HevyClientOptions { | ||||||||||||||||||
|
|
@@ -21,8 +21,8 @@ export function createNodeHevyClientOptions(): HevyClientOptions { | |||||||||||||||||
| "http.method": observation.method, | ||||||||||||||||||
| "http.status_code": observation.status, | ||||||||||||||||||
| "hevy.api.endpoint": observation.endpoint, | ||||||||||||||||||
| ...(getCurrentUserId() | ||||||||||||||||||
| ? { "user.id": getCurrentUserId() as string } | ||||||||||||||||||
| ...(getCurrentUserHash() | ||||||||||||||||||
| ? { "user.hash": getCurrentUserHash() } | ||||||||||||||||||
| : {}), | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🐞 Bug - Double State Read: Capture the return value in a local variable before the spread: const userHash = getCurrentUserHash();
...(userHash ? { "user.hash": userHash } : {}),
Suggested change
Is this review accurate? Use 👍 or 👎 to rate itIf you want to tell us more, use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🐞 Bug - Double Hash Lookup: Capture the result in a local variable and use it in both branches: const userHash = getCurrentUserHash();
...(userHash ? { "user.hash": userHash } : {}),
Suggested change
Is this review accurate? Use 👍 or 👎 to rate itIf you want to tell us more, use |
||||||||||||||||||
| }, | ||||||||||||||||||
| }); | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To ensure that Sentry error events captured during startup validation failures (e.g., when
validateApiKeythrows an error) are also associated with the correct user context, we should seed the Sentry user ID alongside the OpenTelemetry user hash.