mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-09-20 10:03:37 +00:00
* feat(telemetry): report project creation with the id minted for it Acquisition was only observable from a project's first run. That misses every project created and never run, and dates the rest to the wrong day. All three scaffolding paths already mint a project_id into the new pyproject.toml. None of them reported it, and two of them - create_crew and create_json_crew, which is the default `crewai create crew` path - emitted no telemetry at all. `Project Created` carries the kind (crew, json_crew, flow) and the id that was just minted, and is emitted after the mint so it can carry it. The attribute is `created_project_id`, not `project_id`, because those are two different things. CommonAttributesSpanProcessor stamps `project_id` on every span from get_project_id(), which reads the current working directory and is cached for the life of the process - during `crewai create` that describes the directory the command was run from, not the project being created. Reusing the name would have given one column two meanings depending on span type. Nothing is emitted for `create_crew(parent_folder=...)`: that adds a crew to a project which already exists, mints no id, and is not an acquisition. The existing `Flow Creation` span is left exactly as it is. Note for whoever reads it: it is emitted from two places with two different meanings - CLI scaffolding (create_flow.py) and runtime flow construction (event_listener.py on FlowCreatedEvent) - so it cannot separate acquisition from usage. Not changed here because it is a live series and renaming it would break continuity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN * test(telemetry): stop the creation-span tests exporting to the real collector The recorded_spans fixture has to enable telemetry for its assertions to mean anything - a disabled Telemetry never builds self.provider, so every assertion would pass vacuously against a non-recording span. But enabling it is exactly what makes __init__ wire a BatchSpanProcessor around the real SafeOTLPSpanExporter, pointed at the production collector. Measured, with a spy on both SafeOTLPSpanExporter classes reporting at interpreter exit: before this change the file made 3 real export calls, handed 3 synthetic Project Created spans with invented created_project_id values to the production exporter, and completed 3 connects to the collector. After: 0, 0, 0. Sampling at pytest_sessionfinish reports 0 either way and is how this was missed - BatchSpanProcessor flushes on a background timer, and with no provider.shutdown() the flush lands in the atexit handler, which runs after sessionfinish. --block-network does not prevent it: it is function-scoped and only swaps socket.connect, which a background batch thread outlives. Follows telemetry_with_exporter in tests/telemetry/test_tracer_isolation.py: _NullExporter swapped in before construction, _register_shutdown_handlers suppressed so no atexit hook is left behind, and provider.shutdown() in finally. Patch target is crewai_core because that is where this Telemetry comes from. Also disambiguates the docs rows: the minted ID belongs to the new project, not to the directory the command ran in, and the two can differ. Reworded in all four languages rather than renaming the token to created_project_id - this table documents data, never span-attribute keys (kind and crewai_version on the same row are unnamed), and `project_id` is already the page's name for the pyproject.toml key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN * docs(ar): use tanwin fath on the letter, not on the alif مشروعًا / جديدًا rather than مشروعاً / جديداً, in the row added by this PR. Both forms appear in docs/edge/ar (3 each), so this is not a house convention being broken either way; the corrected form is the more standard one and the text is mine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN * docs: stop the creation row implying the span's project_id holds the minted value CodeRabbit re-raised this as Major after I rejected the first version, and it was right to. My rejection argued the page never names wire keys, so introducing created_project_id would be its only one. That part still holds -- grep finds no attribute key anywhere in the four files. But it was the wrong conclusion: the row still used the token `project_id` for a value the span does NOT carry under that key, while the same span's real project_id holds the cwd-derived value. The row also already exposes literal wire values (`crew`, `json_crew`, `flow` are the actual kind values), so "this page has no wire detail" was overstated. Dropping the token resolves the ambiguity without adding the page's only key name: the row now says "the project ID minted for that new project", and names `project_id` only to say the minted value is recorded separately from it. All four languages. No docs/v*/ touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN * docs: use an em dash in the creation row, matching the rest of the page The two ASCII `--` occurrences in these files were both mine, introduced by this PR: the page otherwise uses em dashes throughout (en 4, ar 4, ko 2, pt-BR 4). CodeRabbit flagged pt-BR; the same slip was in en, so both are fixed. Now zero ASCII `--` across all four language files. 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> Co-authored-by: Vidit Ostwal <110953813+Vidit-Ostwal@users.noreply.github.com>
192 lines
7.6 KiB
Python
192 lines
7.6 KiB
Python
"""Scaffolding a project must be observable, and carry the id it just minted.
|
|
|
|
Acquisition was only observable from a project's first *run*. That misses every project
|
|
created and never run, and dates the rest to the wrong day. All three scaffolding paths
|
|
already mint a `project_id` into the new `pyproject.toml`; none of them reported it, and
|
|
two of them emitted no telemetry at all.
|
|
|
|
Lives here rather than in `lib/cli/tests/` because `lib/cli/tests/` is not run by any
|
|
workflow, while this directory is part of the required job.
|
|
"""
|
|
|
|
from typing import Any
|
|
from unittest.mock import patch
|
|
|
|
from click.testing import CliRunner
|
|
from crewai_core.telemetry import Telemetry
|
|
from opentelemetry.sdk.trace.export import SimpleSpanProcessor, SpanExportResult
|
|
from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter
|
|
import pytest
|
|
|
|
|
|
class _NullExporter:
|
|
"""Stands in for the OTLP exporter so no test attempts a real export."""
|
|
|
|
def export(self, spans: Any) -> SpanExportResult:
|
|
return SpanExportResult.SUCCESS
|
|
|
|
def shutdown(self) -> None:
|
|
pass
|
|
|
|
def force_flush(self, timeout_millis: int = 30000) -> bool:
|
|
return True
|
|
|
|
|
|
@pytest.fixture
|
|
def recorded_spans(monkeypatch):
|
|
"""Spans emitted by a live Telemetry, captured from its own provider.
|
|
|
|
Telemetry has to be genuinely enabled: when it is disabled, `__init__` returns before
|
|
building `self.provider` at all, so every assertion here would pass vacuously against
|
|
a non-recording span.
|
|
|
|
Enabling it is exactly what makes the exporter swap mandatory. `__init__` wires a
|
|
`BatchSpanProcessor` around the real `SafeOTLPSpanExporter`, pointed at the production
|
|
collector, so without `_NullExporter` these synthetic `Project Created` spans -- with
|
|
invented `created_project_id` values -- are POSTed to it for real. `--block-network`
|
|
does not prevent that: it is function-scoped and only swaps `socket.connect`, which a
|
|
background batch thread outlives. Same reason `_register_shutdown_handlers` is
|
|
suppressed and the provider is shut down in `finally`: otherwise each test leaves an
|
|
atexit hook and an exporter thread behind.
|
|
|
|
Follows `telemetry_with_exporter` in tests/telemetry/test_tracer_isolation.py; the
|
|
patch target is `crewai_core` because that is where this Telemetry comes from.
|
|
"""
|
|
monkeypatch.setattr(Telemetry, "_instance", None)
|
|
monkeypatch.setattr(Telemetry, "_register_shutdown_handlers", lambda self: None)
|
|
for var in ("OTEL_SDK_DISABLED", "CREWAI_DISABLE_TELEMETRY", "CREWAI_DISABLE_TRACKING"):
|
|
monkeypatch.setenv(var, "false")
|
|
monkeypatch.setattr(
|
|
"crewai_core.telemetry.SafeOTLPSpanExporter",
|
|
lambda **_kwargs: _NullExporter(),
|
|
)
|
|
|
|
telemetry = Telemetry()
|
|
assert telemetry.ready, "telemetry must be live or these assertions prove nothing"
|
|
exporter = InMemorySpanExporter()
|
|
telemetry.provider.add_span_processor(SimpleSpanProcessor(exporter))
|
|
|
|
try:
|
|
yield telemetry, exporter
|
|
finally:
|
|
telemetry.provider.shutdown()
|
|
Telemetry._instance = None
|
|
|
|
|
|
def test_project_created_span_carries_kind_and_the_minted_id(recorded_spans):
|
|
telemetry, exporter = recorded_spans
|
|
|
|
telemetry.project_created_span("crew", "proj-abc123")
|
|
|
|
span = next(s for s in exporter.get_finished_spans() if s.name == "Project Created")
|
|
assert span.attributes["kind"] == "crew"
|
|
assert span.attributes["created_project_id"] == "proj-abc123"
|
|
|
|
|
|
def test_the_minted_id_does_not_reuse_the_project_id_attribute(recorded_spans):
|
|
"""`project_id` and `created_project_id` mean different things and must stay separate.
|
|
|
|
`CommonAttributesSpanProcessor` stamps `project_id` on every span from
|
|
`get_project_id()`, which reads the *current working directory* and is cached for the
|
|
life of the process. During `crewai create` that is the directory the command was run
|
|
from, not the project being created. Writing the minted id into the same attribute
|
|
would silently give one column two meanings.
|
|
"""
|
|
telemetry, exporter = recorded_spans
|
|
|
|
telemetry.project_created_span("flow", "proj-new")
|
|
|
|
span = next(s for s in exporter.get_finished_spans() if s.name == "Project Created")
|
|
assert span.attributes["created_project_id"] == "proj-new"
|
|
assert span.attributes.get("project_id") != "proj-new", (
|
|
"the minted id must not overwrite the cwd-derived project_id"
|
|
)
|
|
|
|
|
|
def test_a_failed_mint_records_an_empty_string_not_a_missing_key(recorded_spans):
|
|
"""`get_or_create_project_id` returns None when pyproject is missing or unwritable.
|
|
|
|
Empty rather than absent, matching the convention established for the common
|
|
`project_id` attribute: absent means an old client that never sent the key at all,
|
|
which is a different fact from a client that sent it and had nothing to report.
|
|
"""
|
|
telemetry, exporter = recorded_spans
|
|
|
|
telemetry.project_created_span("crew", None)
|
|
|
|
span = next(s for s in exporter.get_finished_spans() if s.name == "Project Created")
|
|
assert span.attributes["created_project_id"] == ""
|
|
assert "created_project_id" in span.attributes
|
|
|
|
|
|
SCAFFOLDERS = [
|
|
("crew", "crewai_cli.create_crew", "create_crew", {"skip_provider": True}),
|
|
(
|
|
"json_crew",
|
|
"crewai_cli.create_json_crew",
|
|
"create_json_crew",
|
|
{"skip_provider": True},
|
|
),
|
|
("flow", "crewai_cli.create_flow", "create_flow", {}),
|
|
]
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("kind", "module", "func_name", "kwargs"),
|
|
SCAFFOLDERS,
|
|
ids=[s[0] for s in SCAFFOLDERS],
|
|
)
|
|
def test_each_scaffolder_reports_the_id_it_minted(
|
|
kind, module, func_name, kwargs, monkeypatch
|
|
):
|
|
"""The span must carry the value the mint returned, which pins the ordering.
|
|
|
|
Asserting on the minted value rather than merely that the span was emitted is what
|
|
makes this a test of ordering: a span emitted before the mint could not carry it.
|
|
`create_flow` already emitted `flow_creation_span` *before* minting, so this is a
|
|
real mistake to guard against and not a hypothetical one.
|
|
"""
|
|
import importlib
|
|
|
|
# The product's own non-interactive mode. `create_json_crew` otherwise runs an agent
|
|
# wizard that reads stdin; this is the supported way to skip it rather than a stub.
|
|
monkeypatch.setenv("CREWAI_DMN", "1")
|
|
|
|
mod = importlib.import_module(module)
|
|
func = getattr(mod, func_name)
|
|
|
|
with CliRunner().isolated_filesystem():
|
|
with (
|
|
patch.object(mod, "get_or_create_project_id", return_value="proj-minted"),
|
|
patch.object(mod, "Telemetry") as telemetry_cls,
|
|
):
|
|
func("demo_project", **kwargs)
|
|
|
|
telemetry_cls.return_value.project_created_span.assert_called_once_with(
|
|
kind, "proj-minted"
|
|
)
|
|
|
|
|
|
def test_adding_a_crew_to_an_existing_project_is_not_an_acquisition():
|
|
"""`create_crew(parent_folder=...)` adds a crew to a project that already exists.
|
|
|
|
It mints no id and must emit no creation span, or every added crew would be counted
|
|
as a newly acquired project.
|
|
"""
|
|
from crewai_cli import create_crew as module
|
|
|
|
with CliRunner().isolated_filesystem():
|
|
module.create_folder_structure("host_project")
|
|
with (
|
|
patch.object(module, "get_or_create_project_id") as mint,
|
|
patch.object(module, "Telemetry") as telemetry_cls,
|
|
):
|
|
module.create_crew(
|
|
"extra_crew", skip_provider=True, parent_folder="host_project"
|
|
)
|
|
|
|
assert not mint.called, "the parent_folder branch must not mint a new id"
|
|
assert not telemetry_cls.return_value.project_created_span.called, (
|
|
"adding a crew to an existing project must not be reported as a new project"
|
|
)
|