feat(otel): tag signals with enduser.id for multi-developer attribution#52
feat(otel): tag signals with enduser.id for multi-developer attribution#52danielemoraschi wants to merge 2 commits into
Conversation
Auto-detect the OS username via os.userInfo() and tag every metric datapoint, log record, and trace span (resource and span attributes) with `enduser.id`. Refine from opencode's `cfg.username` once the config hook fires. Signal-level attribution (not resource-only) is required because Datadog's direct OTLP intake promotes only a hardcoded set of resource attributes to tags and silently drops the rest. OTEL_RESOURCE_ATTRIBUTES overrides still win on the resource. Opt out with OPENCODE_DISABLE_USER_TRACKING. When os.userInfo() throws, the attribute is omitted entirely rather than tagged with a placeholder.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds optional user identity tracking to OpenTelemetry instrumentation by deriving ChangesUser identity tracking feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/util.test.ts`:
- Around line 12-16: Make the success-path test for safeUsername deterministic
by mocking os.userInfo in the test: use a jest spy/mock on os.userInfo to return
an object with a username (e.g. "testuser") before calling safeUsername in the
"returns the OS username on success" test, assert against the returned string as
now, and restore/clear the mock after the test so other tests are unaffected;
reference the safeUsername helper and the test in tests/util.test.ts when adding
the mock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 65790093-68dc-4975-8602-4ee740a6c2ec
📒 Files selected for processing (10)
AGENTS.mdREADME.mdsrc/config.tssrc/index.tssrc/otel.tssrc/types.tssrc/util.tstests/config.test.tstests/otel.test.tstests/util.test.ts
The original test called the real `os.userInfo()` and asserted on its return, which fails in minimal containers (e.g. distroless / Alpine without a passwd entry) where `os.userInfo()` throws and `safeUsername` returns `undefined`. Mock the success case so the test is deterministic across environments, matching the pattern already used by the error-path test.
|
I like the problem this is solving, especially for shared backends where per-developer attribution is genuinely useful. My hesitation is mostly about hardcoding Would it make more sense to generalize this slightly and make the behavior resource-attribute driven instead? Something along the lines of:
The idea would be:
That seems like it would still solve the Datadog-style compatibility issue, while keeping the plugin more general-purpose and avoiding a growing list of bespoke propagated fields later. I’d strongly prefer that we not introduce single hardcoded propagated attributes into the plugin, since I think that sets us up for more one-off exceptions over time. If you’d rather not take that on in this PR, I’d be perfectly happy for this one to be closed and for us to tackle the generalized version instead. |
2 participants
Description
Adds an
enduser.idattribute to every metric datapoint, log record, and trace span (both the resource and per-span attributes) so telemetry exported to a shared OTLP backend can be attributed to the developer who generated it. The value is auto-detected fromos.userInfo().usernameand refined from opencode'scfg.usernameconfig field once theconfighook fires. SetOPENCODE_DISABLE_USER_TRACKINGto any non-empty value to opt out.enduser.idis added at the signal level (datapoint / record / span attributes), not just on the OTel resource, because Datadog's direct OTLP intake (otlp.datadoghq.com/otlp.datadoghq.eu) promotes only a hardcoded set of well-known resource attributes to tags and silently drops the rest — so resource-only attribution shows up as "no tag" in the Datadog Metrics Summary. Datapoint-level attributes are always preserved across backends. Override precedence is unchanged:OPENCODE_RESOURCE_ATTRIBUTES=enduser.id=…still wins on the resource. Whenos.userInfo()throws (e.g. containerised runs with no passwd entry), the attribute is omitted entirely rather than tagged with a placeholder like"unknown".See the new "User identity tracking" section in
README.mdfor the full propagation table and opt-out details.Type of change
Checklist
bun run lintpasses with no errorsbun run check:jsdoc-coveragepasses with no errors (85.11%, 40/47)bun run typecheckpasses with no errorsbun testpasses with no errors (251 pass / 0 fail)Related issues
None.
Additional context
Tests added (8 total):
tests/config.test.ts—disableUserTrackingdefaults tofalse; becomestrueon any non-empty env value.tests/otel.test.ts—buildResource(version, endUserId)includes / omitsenduser.id;OTEL_RESOURCE_ATTRIBUTES=enduser.id=…overrides the seeded value.tests/util.test.ts—safeUsername()returns the OS username on success andundefinedwhenos.userInfo()throws.End-to-end verification suggested before merge (not covered by unit tests):
OPENCODE_ENABLE_TELEMETRY=1and confirmenduser.id=<OS user>appears on a metric datapoint, a log record, and a span resource.OPENCODE_DISABLE_USER_TRACKING=1and confirm the attribute is absent on all three signals.OPENCODE_RESOURCE_ATTRIBUTES=enduser.id=overrideand confirm the resource showsoverridewhile signals show the OS user.username: "bob"in opencode config and confirm metrics / logs flip tobobafter the config hook fires; the trace resource remains the OS user (documented quirk).Summary by CodeRabbit
New Features
OPENCODE_DISABLE_USER_TRACKINGenvironment variable to opt outDocumentation