[ashwmu]

Problem

TjHttpExporter.export() was hardcoding service.name="tokenjam" in the OTLP payload sent to tj serve, ignoring the gen_ai.agent.id set on spans by @watch(). Since tj falls through to service.name when gen_ai.agent.id is absent on the wire, every external agent appeared as "tokenjam" in tj status.

Root cause

@watch(agent_id="my-agent") was used correctly in tokenjam-agents. The issue was that http_exporter.py was overwriting the agent identity with a hardcoded literal before sending to tj serve.

Fix

Following the pattern in cloud_sync.py:571: group spans by gen_ai.agent.id and derive service.name from that. Falls back to "tokenjam" when no agent id is present. The OTEL_SERVICE_NAME approach from the first commit is dropped - @watch() is the correct and documented path.

Changes

• tokenjam/sdk/http_exporter.py - derives service.name from gen_ai.agent.id
• tests/unit/test_http_exporter_auth.py- 3 new tests covering agent_id present, absent, and mixed agents

Related issue

No existing issue - discovered while building tokenjam-agents reference implementation.

Checklist

• Tests pass (pytest tests/unit/ tests/synthetic/ tests/agents/ tests/integration/)
• Lint clean (ruff check tokenjam/)
• Type check clean (mypy tokenjam/)
• CLAUDE.md updated (if architecture changed)
• Test spans use tests/factories.py (not raw NormalizedSpan)
• Requested @anilmurty as reviewer (or @-mentioned him above)

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; the previous test-isolation issue is fixed.

Summary

The PR sends gen_ai.agent.id as service.name, then falls back to the resource's declared name and finally tokenjam.

• The latest changes keep naming tests independent of the developer's environment.
• Configured-name tests still set their own values after setup.
• The numbered previous finding is fixed. No new actionable issues were found.
• Review checks were limited to reading code; tests were not run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Completed span] --> B{Has gen_ai.agent.id?}
  B -->|Yes| C[Use agent ID]
  B -->|No| D{Has declared service.name?}
  D -->|Yes| E[Use declared name]
  D -->|No| F[Use tokenjam]
  C --> G[Group spans by chosen name]
  E --> G
  F --> G
  G --> H[Send OTLP payload]

Loading

_{Reviews (4) · Last reviewed commit: "Stop the service-name tests reading the ..." · Reviewed by Greptile}

[anilmurty]

Thanks @ashwmu — the problem you've found is real, and the fallback you're relying on is real too: otlp_parsing.py:154 does use service.name as the agent_id when gen_ai.agent.id is absent. But I think the change needs rework in two places, and there's a design question worth settling before either.

1. Blocking: the bootstrap.py half has no effect

Greptile called this and it checks out. TjHttpExporter.export() builds its OTLP payload from scratch and hardcodes the resource (http_exporter.py:46). It never reads span.resource, so setting the resource on the TracerProvider changes nothing on the wire.

I ran it rather than reading it. Building the resource exactly as _try_http_mode does after this patch, with OTEL_SERVICE_NAME=my-agent, then capturing the POST body:

resource on the provider : my-agent
service.name ON THE WIRE : tokenjam

Your PR body says the bug is "when tj serve is running and HTTP mode is used", so this is the exact path you're targeting. As it stands that half of the diff is dead code. The fix is in http_exporter.py: send the span's own resource attributes instead of a literal.

2. It only covers one of the two env vars

os.environ.get("OTEL_SERVICE_NAME") handles one spelling. OTEL_RESOURCE_ATTRIBUTES=service.name=my-agent is the other standard one, and it still lands on "tokenjam":

OTEL_SERVICE_NAME=my-agent                         -> my-agent     ✓
OTEL_RESOURCE_ATTRIBUTES=service.name=my-agent     -> tokenjam     ✗

Worth knowing: _build_tj_resource already solves this problem for service.version, and its docstring spells out the pattern. Resource.create() detects both env vars for you; the explicit dict is what overrides them. So the shape that matches the file's own convention is to let the detector run and only fill in a default when nothing was detected:

resource = Resource.create()
if resource.attributes.get("service.name") in (None, "unknown_service"):
    resource = resource.merge(Resource({"service.name": "tokenjam"}))

I tried it. All four cases come out right, including OTEL_SERVICE_NAME winning over OTEL_RESOURCE_ATTRIBUTES when both are set, which is what the spec requires and what os.environ.get cannot do on its own.

3. The design question

Before we add a second way to name an agent, is this the knob we want?

The supported path today is @watch(agent_id="my-agent"), and patch_anthropic() inherits from it (integrations/anthropic.py:86, "Inherit agent_id from parent span"). Your symptom, every external agent showing as tokenjam, is what happens when patch_*() is called with no @watch() wrapper above it.

So there are two readings. If the agent should be wrapped, the fix is documentation, not code. If a process can legitimately be one agent end to end with no per-request wrapper, that's a real gap, but OTEL_SERVICE_NAME is an odd place to put the answer: in OTel it names the service, and we would be quietly giving it a second meaning. A [sdk] agent_id config key, or an argument on the bootstrap call, would be explicit and discoverable.

There is precedent on your side. otlp_parsing.py:152 says "OpenClaw sets service.name", so we already read it that way for one producer. I'm not against it. I'd just like the decision made deliberately and written down, because right now nothing documents that OTEL_SERVICE_NAME renames your agent, and the next person will hit the same confusion from the other direction.

Which did you run into? If @watch() was impractical in tokenjam-agents, say why and that settles it.

4. No test

Every fix here ships with a test that fails without it. tests/unit/test_provider_attribution.py is the natural home, and _build_tj_resource was pulled out specifically to be unit-testable. A case per env var, plus one asserting the HTTP exporter puts the resolved name on the wire, would have caught both findings above.

CI is green, which is worth noting for the opposite reason: nothing in the suite covers either path.

Happy to take this the rest of the way if you'd rather hand it over. The problem is worth fixing.

[anilmurty]

Adding two things: a plainer summary of what I'm asking for, and something you can't see from this repo that changes the stakes on the design question.

The plain version

Every span that reaches tj has to say which agent it came from. tj looks in two places, in order: a field called gen_ai.agent.id, and if that's empty, service.name.

The SDK's way to fill in the first one is @watch("my-agent") on your entry function. patch_anthropic() then copies that name onto every LLM call it wraps (integrations/anthropic.py:86). You called patch_anthropic() without @watch(), so nothing filled in the first field, tj fell through to the second, and service.name is the hardcoded string "tokenjam". That's why every agent showed up with our name. Your diagnosis was right.

What the PR misses is that one of the two files you changed has no effect.

In HTTP mode the thing that actually sends data to tj serve is TjHttpExporter.export(). It doesn't read the resource you set on the provider. It builds its own message and types "tokenjam" into it by hand (http_exporter.py:46). So it's like writing an address on a parcel and having the shipping desk ignore it and write a different one on the label.

I checked rather than assuming. Building the resource exactly as your patched _try_http_mode does, with OTEL_SERVICE_NAME=my-agent, then capturing the outgoing POST:

resource on the provider : my-agent
service.name on the wire : tokenjam

Your PR description says the bug is "when tj serve is running and HTTP mode is used". That is this path. So the bootstrap.py half of the diff changes nothing for the case you set out to fix. That's the main thing I want addressed: http_exporter.py should send the span's own resource instead of a literal.

The second thing is smaller. os.environ.get("OTEL_SERVICE_NAME") covers one of the two standard ways to set a service name; OTEL_RESOURCE_ATTRIBUTES=service.name=my-agent still gives you "tokenjam". _build_tj_resource already handles both correctly for service.version, a few lines below your change, and its docstring explains how. Following that pattern covers all the cases and costs you nothing.

Why the design question matters more than it looks

Here's the part you can't see from this repo. In OSS, service.name is a fallback for agent id. In Cloud it is agent identity, and a unique key: services is UNIQUE (org_id, service_name), and the spans table stores agent_id TEXT -- service.name.

The OSS install forwards to Cloud through core/cloud_sync.py, which sets service.name from each span's agent_id and posts it. So whatever an agent is called locally becomes its permanent identity row in that org's Cloud account.

Which means if we ship this as-is, someone who sets OTEL_SERVICE_NAME=checkout-api for perfectly ordinary OpenTelemetry reasons, with no idea tj reads it, would create or rename an agent in their Cloud org. That is a surprising amount of reach for an environment variable whose documented meaning is just "the name of this service".

This is the argument for an explicit knob instead: a [sdk] agent_id config key, or an argument on the bootstrap call. It does the same job, it can't be set by accident, and it does not give a standard OTel variable a second private meaning.

I'm still open to the env var if you hit a case where @watch() genuinely did not fit. Tell me what that case was and I'll take the decision from there.

One more thing worth stealing: cloud_sync.py:571 already does exactly what I'm asking http_exporter.py to do, deriving the resource from the span rather than hardcoding it. Same job, two implementations, and only one of them got it right. Worth a look before you rework.

[ashwmu]

Thanks for the detailed walkthrough.
To answer your question: @watch(agent_id="daily-briefing-agent") was used on the entry function in tokenjam-agents. The symptom tj status showing tokenjam - was caused exactly as you described: http_exporter.py was hardcoding "tokenjam" in the outgoing payload, so the gen_ai.agent.id set by @watch() never reached tj serve as service.name.

The OTEL_SERVICE_NAME approach was the wrong diagnosis. I agree it shouldn’t be shipped - the Cloud identity risk you described (permanent agent rows, UNIQUE constraint) makes the env var dangerous for anyone using it for its documented OTel meaning.

Updated the PR:
Reverted bootstrap.py and provider.py — those changes had no effect and are gone
Fixed http_exporter.py following cloud_sync.py:571 — groups spans by gen_ai.agent.id and derives service.name from that, falling back to "tokenjam"
Will add tests to test_provider_attribution.py next

[anilmurty]

Thanks for the quick turnaround @ashwmu. The exporter fix is a real improvement and I want to keep it. But the PR still does not fix the thing it is named after, and the new test now pins the bug in place.

The headline symptom is unchanged

I ran both paths against your branch, with OTEL_SERVICE_NAME=my-agent set:

provider resource says: my-agent

@watch used (agent.id present)                 service.name=['my-agent']   agent.id on span=True
patch_* only, no @watch (the reported bug)     service.name=['tokenjam']   agent.id on span=False

The second row is your PR description, verbatim: "agent always showing as 'tokenjam' in tj status when using patch_anthropic() or other SDK integrations from external projects." It still says tokenjam. OTEL_SERVICE_NAME is set and still ignored, because the resource _try_http_mode builds is still never read by anything.

So the bootstrap.py half of the diff remains dead code, which is what Greptile is still flagging too.

And the first row does nothing either

When gen_ai.agent.id is present, the parser reads it straight off the span attributes (otel/otlp_parsing.py:148) and never consults service.name at all. The fallback on line 154 only fires when agent_id is missing.

So the new grouping is a no-op in both directions: where it changes service.name, nothing reads it; where something would read it, it still says tokenjam.

I am not asking you to revert it. Grouping by agent and carrying a real name is the correct shape, it matches what the Cloud bridge already does in core/cloud_sync.py:571, and it matters the moment these spans reach a receiver that keys on service.name. It is just not the fix.

The test pins the bug

def test_export_falls_back_to_tokenjam_when_no_agent_id() -> None:
    """Spans without gen_ai.agent.id fall back to service.name=tokenjam."""
    ...
    assert attrs["service.name"] == "tokenjam"

That asserts the exact behaviour you opened the PR to change. Critical Rule 23 in CLAUDE.md is about this shape: a green suite that enforces the defect, so the next person to do the right thing is told by CI that they broke something.

The repair is to invert it rather than delete it. The case is worth a test; the expected value is wrong. It should assert that a span with no agent_id picks up the configured service name, and fall back to tokenjam only when nothing is configured.

There is also no test anywhere for OTEL_SERVICE_NAME, which is the one behaviour the PR is named after.

What would actually close this

The exporter needs to fall back to the resource, not to a literal:

agent_id = str(attrs.get("gen_ai.agent.id") or "")
if not agent_id:
    agent_id = str((span.resource.attributes or {}).get("service.name") or "tokenjam")

That makes the bootstrap.py half live, and OTEL_SERVICE_NAME finally reaches the wire. Keep your grouping exactly as it is.

While you are in provider.py: os.environ.get("OTEL_SERVICE_NAME") still misses OTEL_RESOURCE_ATTRIBUTES=service.name=…, and _build_tj_resource's own docstring explains the pattern that handles both, which it already applies to service.version ten lines below your change.

One thing to settle before more code

I raised this last time and it is still open, and it decides how much of the above is worth doing.

The supported way to name an agent is @watch(agent_id=...), which patch_anthropic() inherits from (integrations/anthropic.py:86). Your symptom is what happens when patch_*() runs with no @watch() above it.

So: did you hit a case where @watch() genuinely did not fit in tokenjam-agents? If yes, that is a real gap and I would rather close it with an explicit knob than by overloading an OpenTelemetry variable. One more reason, which you cannot see from this repo: in Cloud, service.name is agent identity, stored under a unique constraint per org. Anyone who sets OTEL_SERVICE_NAME for ordinary OTel reasons would be creating or renaming an agent in their org without knowing it.

If @watch() would have worked, the fix is a docs line and we close this.

Tell me which and I will take it from there, or finish it myself if you would rather hand it over.

[anilmurty]

Took this the rest of the way, @ashwmu — pushed 07ad935. Your grouping is kept; I changed what it falls back to.

The reframe that settles the design question

I kept asking whether we should give OTEL_SERVICE_NAME a second meaning. Wrong question. We were already overriding it.

Resource.create({"service.name": ...}) merges the explicit dict over whatever the detector read from the environment. So passing that key at all is what made the variable inert. We were not failing to support a standard mechanism; we were actively suppressing one.

That makes this a plain bug fix, not a new knob, and it removes my objection. Nothing is being overloaded.

It also explains why reading one variable by hand only got halfway: OTEL_RESOURCE_ATTRIBUTES=service.name=... stayed ignored, and one os.environ.get cannot express which wins when both are set.

The fix is to stop passing the key, let the detector run, and default to tokenjam only when it found nothing. That is the same shape _build_tj_resource already used for service.version, for the reason its own docstring gives.

nothing set                                     -> tokenjam
OTEL_SERVICE_NAME=my-agent                      -> my-agent
OTEL_RESOURCE_ATTRIBUTES=service.name=my-agent  -> my-agent
both set                                        -> from-name   (spec precedence)
OTEL_RESOURCE_ATTRIBUTES=service.version=2.3.1  -> service.version still honoured

The exporter

Your grouping by gen_ai.agent.id stays. I only changed the fallback from a literal to the span's resource, which is what makes the bootstrap.py half live:

patch_* with no @watch, OTEL_SERVICE_NAME=my-agent
  before: service.name=tokenjam
  after:  service.name=my-agent

Identity is now most-specific-first: the span's own agent id, then the declared service name, then tj's own.

The test

test_export_falls_back_to_tokenjam_when_no_agent_id asserted service.name == "tokenjam" for a span with no agent id — the exact symptom the PR was opened to remove. Left alone it would have told the next person their correct fix was a regression.

Inverted rather than deleted, per Critical Rule 23. The case was worth testing; the expected value was wrong. It now asserts the declared name is used, with separate tests for the genuinely-undeclared fallback and for an explicit agent id still winning.

Each new test was checked against the half of the fix it covers by reverting that half and watching it go red.

Still worth doing, separately

@watch() is the discoverable path and nothing points at it. Your symptom came from calling patch_anthropic() with no @watch() above it, and nothing in the docs or the CLI says that is what names your agent. This PR means a service that sets OTEL_SERVICE_NAME is no longer silently renamed, but the better answer for most people is still @watch. Worth a docs line, and I will file it.

Cloud, for context you cannot see from this repo: service.name is agent identity there, under a unique constraint per org. That makes honouring the standard variables more correct, not less — a service that declares its own name now keeps it end to end instead of collapsing into a shared tokenjam row.

ruff and mypy clean, 6,021 tests pass. The three summarize failures under -n auto are pre-existing cross-test leakage, identical on main and green in isolation.

Thanks for pushing on this one — the exporter grouping was the right instinct and it stays.

Resolved. The exporter now falls back to the span's resource, the provider lets the OTel detector resolve service.name (both standard env vars, correct precedence), the test that pinned the symptom is inverted, and the ambient-env dependency Greptile found is fixed with an autouse fixture. CI 6/6, Greptile 5/5 with no actionable issues.

Dismissing my own request rather than converting it to an approval: the last two commits are mine, so the approving review should not also be. Over to @anilmurty.