fix(events): always emit project_id so absent and empty stay distinct (#7056)

* fix(telemetry): always emit project_id so absent and empty stay distinct

common_span_attributes() stamped project_id onto every span only when the project
declared one, and omitted the key otherwise. That makes two different situations
indistinguishable downstream: a client too old to report a project id at all, and
a current client whose project simply declares none.

The consequence is not cosmetic. The share of clients that COULD have reported an
id is the denominator of every attribution rate, and with both cases collapsed into
"key absent" that denominator cannot be computed at all - it can only be inferred
from a version floor, which is fragile and silently wrong for any client that
backports or pins.

The key is now always present and is the empty string when the project declares
none. It still never invents an identity: get_project_id() remains read-only and
minting stays with the CLI commands a user explicitly invoked.

Two existing tests asserted the old contract and are updated rather than deleted,
one of them renamed because its name described the behaviour that changed. A third
test is added pinning the distinction itself. The test asserting that a foreign
application's spans are never annotated is unaffected and still passes: this
changes what our processor stamps, not where it is attached.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN

* docs(telemetry): note that a failed project_id lookup also yields empty

Addresses CodeRabbit on #7056. The docstring and inline comment described the
empty string only as an undeclared project, but the except branch sets
project_id to None and so lands on the same empty value. Both are deliberately
indistinguishable - neither yields an id - and saying so matters to anyone
debugging an empty value, since an unreadable pyproject.toml looks identical to
a project that simply declares nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
João Moura
2026-08-20 01:10:23 -03:00
committed by GitHub
parent b3ab193c31
commit 7c72d57b73
2 changed files with 47 additions and 9 deletions

View File

@@ -135,8 +135,11 @@ def common_span_attributes() -> dict[str, str]:
imports ``crewai``, still reports the same process-wide context.
Returns:
Attributes to stamp on every span. ``project_id`` is omitted for
projects that do not declare one.
Attributes to stamp on every span. ``project_id`` is always present and
is the empty string whenever no id is available -- both for projects that
declare none and when the lookup itself failed, which are deliberately
indistinguishable here because neither yields an id. See the comment at
the assignment for why empty and *absent* must stay distinct.
"""
attributes = {
"coding_agent": detect_coding_agent(),
@@ -151,8 +154,15 @@ def common_span_attributes() -> dict[str, str]:
logger.debug("Failed to read project id: %s", e)
project_id = None
if project_id:
attributes["project_id"] = project_id
# Always set the key, even when empty. Absent and empty mean different things
# and only this distinction can tell them apart: absent means the client is too
# old to report a project id at all, empty means the client asked and the project
# declares none -- or the lookup failed, which lands here too and is treated the
# same, since an unreadable pyproject.toml also means no id is available.
# Collapsing empty into "absent" makes the share of clients that COULD have
# reported one unknowable, and that share is the denominator every attribution
# rate needs.
attributes["project_id"] = project_id or ""
return attributes

View File

@@ -547,15 +547,27 @@ def test_common_attributes_include_project_id_when_declared(clean_env, monkeypat
assert attributes["project_id"] == "proj-123"
def test_project_id_is_omitted_when_absent(clean_env, monkeypatch):
"""Projects without an id must not report a placeholder."""
def test_project_id_is_empty_rather_than_absent_when_undeclared(clean_env, monkeypatch):
"""An undeclared project reports an EMPTY id, not a missing key.
This used to assert the key was omitted. It is now always present, because
absent and empty answer different questions and only the caller can tell them
apart: a missing key means the client is too old to report a project id at all,
an empty one means the client asked and the project declares none. With both
collapsed into "absent", the share of clients that COULD have reported an id is
unknowable -- and that share is the denominator any attribution rate needs.
"""
attributes = _common_attributes(monkeypatch, project_id=None)
assert "project_id" not in attributes
assert attributes["project_id"] == ""
assert "project_id" in attributes, (
"the key must be present even when empty, or absent and undeclared "
"become indistinguishable"
)
def test_project_id_lookup_never_breaks_telemetry(clean_env, monkeypatch):
"""A failed lookup degrades to omitting the attribute."""
"""A failed lookup degrades to an empty id, and never to a raised exception."""
from crewai_core.telemetry import common_span_attributes
def boom(*args, **kwargs):
@@ -566,10 +578,26 @@ def test_project_id_lookup_never_breaks_telemetry(clean_env, monkeypatch):
attributes = common_span_attributes()
assert "project_id" not in attributes
assert attributes["project_id"] == ""
assert "coding_agent" in attributes
def test_a_declared_id_is_distinguishable_from_an_undeclared_one(
clean_env, monkeypatch
):
"""The whole point of the change: the two cases produce different values.
Both are present, so a reader can always tell "no project" from "old client",
which is what makes an attribution denominator computable.
"""
declared = _common_attributes(monkeypatch, project_id="proj-123")
undeclared = _common_attributes(monkeypatch, project_id=None)
assert declared["project_id"] == "proj-123"
assert undeclared["project_id"] == ""
assert declared["project_id"] != undeclared["project_id"]
def test_common_attributes_are_computed_once(clean_env, monkeypatch):
"""The project file must not be re-read for each provider."""
from crewai_core.telemetry import common_span_attributes