Add structured telemetry adapters with in-memory references and tests - #17
Conversation
![terragon-labs[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
Introduce example telemetry adapters in cuprum.adapters: - logging_adapter: structured logging hook with JSON formatter for detailed exec event logs - metrics_adapter: Prometheus-style metrics hook with counters and histograms via protocol - tracing_adapter: OpenTelemetry-style tracing hook managing spans with attributes and events All adapters use protocol classes to remain dependency-free and non-blocking. Added comprehensive unit and behaviour tests, documentation updates, and usage examples for easy integration and extension. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 0 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughSummarise the addition of a telemetry adapter suite for Cuprum: structured logging, Prometheus‑style metrics and OpenTelemetry‑style tracing adapters, with protocol‑based backends, in‑memory reference implementations, factories returning ExecHook callables, unit and behaviour tests, and design and user documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Exec as Executor (Cuprum)
participant Hook as ExecHook
participant Logger as Logger
participant Metrics as MetricsCollector
participant Tracer as Tracer
rect rgb(235,245,255)
Exec->>Hook: plan(event)
Hook->>Logger: emit plan log (cuprum_phase=plan)
end
rect rgb(235,255,235)
Exec->>Hook: start(event)
Hook->>Metrics: inc_counter(cuprum_executions_total, labels)
Hook->>Tracer: start_span("cuprum.exec {program}", attributes)
Hook->>Logger: emit start log (cuprum_phase=start)
end
rect rgb(255,250,235)
Exec->>Hook: stdout(event)
Hook->>Logger: emit stdout log (cuprum_phase=stdout)
Hook->>Metrics: inc_counter(cuprum_stdout_lines_total, ...)
Hook->>Tracer: add_event("stdout", data)
end
rect rgb(255,235,235)
Exec->>Hook: stderr(event)
Hook->>Logger: emit stderr log (cuprum_phase=stderr)
Hook->>Metrics: inc_counter(cuprum_stderr_lines_total, ...)
Hook->>Tracer: add_event("stderr", data)
end
rect rgb(235,235,255)
Exec->>Hook: exit(event)
Hook->>Metrics: observe_histogram(cuprum_duration_seconds, duration)
Hook->>Metrics: inc_counter(cuprum_failures_total) if exit_code != 0
Hook->>Tracer: set_status / end_span (exit_code, duration)
Hook->>Logger: emit exit log (cuprum_phase=exit)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
Reviewer's GuideAdds a new cuprum.adapters package with protocol-based, optional telemetry adapters for structured logging, Prometheus-style metrics, and OpenTelemetry-style tracing, including in-memory reference implementations, associated observe hooks/factories, behavioural/unit tests, and documentation updates describing design and usage. Sequence diagram for sh.observe with telemetry adapterssequenceDiagram
actor User
participant ScopedContext as scoped
participant Shell as sh
participant ExecEngine as ExecEngine
participant LoggingHook as structured_logging_hook
participant MetricsHook as MetricsHook
participant TracingHook as TracingHook
participant Logger as Logger
participant MetricsBackend as MetricsCollector
participant TracerBackend as Tracer
User->>ScopedContext: scoped(allowlist)
activate ScopedContext
ScopedContext->>Shell: sh.observe(LoggingHook, MetricsHook, TracingHook)
activate Shell
User->>Shell: make(ECHO)("hello").run_sync()
Shell->>ExecEngine: execute command
loop ExecEvent stream
ExecEngine-->>Shell: ExecEvent(phase, ...)
Shell->>LoggingHook: __call__(ExecEvent)
alt Logging enabled for phase
LoggingHook->>Logger: log(level, message, extra)
end
Shell->>MetricsHook: __call__(ExecEvent)
MetricsHook->>MetricsBackend: inc_counter/observe_histogram
Shell->>TracingHook: __call__(ExecEvent)
alt phase == start
TracingHook->>TracerBackend: start_span(name, attributes)
else phase == stdout or phase == stderr
TracingHook->>TracerBackend: Span.add_event(name, attributes)
else phase == exit
TracingHook->>TracerBackend: Span.set_attribute(exit_code, duration_s)
TracingHook->>TracerBackend: Span.set_status(ok)
TracingHook->>TracerBackend: Span.end()
end
end
ExecEngine-->>Shell: completion
deactivate Shell
ScopedContext-->>User: exit context
deactivate ScopedContext
Class diagram for structured logging adapterclassDiagram
direction LR
class ExecEvent {
<<external>>
phase: str
program: object
argv: Sequence[object]
tags: Mapping[str, object]
pid: int
cwd: object
exit_code: int
duration_s: float
line: str
}
class ExecHook {
<<external>>
__call__(event: ExecEvent) None
}
class LogLevels {
plan_level: int
start_level: int
output_level: int
exit_level: int
}
class structured_logging_hook {
+structured_logging_hook(logger: Logger, levels: LogLevels) ExecHook
}
class JsonLoggingFormatter {
+format(record: LogRecord) str
}
class Logger {
<<external>>
+isEnabledFor(level: int) bool
+log(level: int, msg: str, extra: dict~str, object~) None
}
class LogRecord {
<<external>>
}
class logging_adapter_module {
_DEFAULT_LOGGER_NAME: str
+_build_extra(event: ExecEvent) dict~str, object~
+_format_message(event: ExecEvent) str
+_json_serializable(value: object) object
}
ExecHook <|.. structured_logging_hook
JsonLoggingFormatter --|> Formatter
Logger <.. structured_logging_hook
LogLevels <.. structured_logging_hook
class Formatter {
<<external>>
+format(record: LogRecord) str
}
Class diagram for metrics and tracing adaptersclassDiagram
direction LR
class ExecEvent {
<<external>>
phase: str
program: object
argv: Sequence[object]
tags: Mapping[str, object]
pid: int
cwd: object
exit_code: int
duration_s: float
line: str
}
class ExecHook {
<<external>>
__call__(event: ExecEvent) None
}
%% Metrics side
class MetricsCollector {
<<protocol>>
+inc_counter(name: str, value: float, labels: Mapping~str, str~) None
+observe_histogram(name: str, value: float, labels: Mapping~str, str~) None
}
class InMemoryMetrics {
counters: dict~str, float~
histograms: dict~str, list~float~~
_lock: Lock
+inc_counter(name: str, value: float, labels: Mapping~str, str~) None
+observe_histogram(name: str, value: float, labels: Mapping~str, str~) None
+reset() None
}
class MetricsHook {
-_collector: MetricsCollector
+__init__(collector: MetricsCollector)
+__call__(event: ExecEvent) None
+_extract_labels(event: ExecEvent) dict~str, str~
}
class metrics_hook {
+metrics_hook(collector: MetricsCollector) ExecHook
}
MetricsCollector <|.. InMemoryMetrics
ExecHook <|.. MetricsHook
MetricsCollector <.. MetricsHook
metrics_hook ..> MetricsHook
%% Tracing side
class Span {
<<protocol>>
+set_attribute(key: str, value: object) None
+add_event(name: str, attributes: Mapping~str, object~) None
+set_status(ok: bool) None
+end() None
}
class Tracer {
<<protocol>>
+start_span(name: str, attributes: Mapping~str, object~) Span
}
class InMemorySpan {
name: str
attributes: dict~str, object~
events: list~tuple~str, dict~str, object~~~~
status_ok: bool
ended: bool
+set_attribute(key: str, value: object) None
+add_event(name: str, attributes: Mapping~str, object~) None
+set_status(ok: bool) None
+end() None
}
class InMemoryTracer {
spans: list~InMemorySpan~
_lock: Lock
+start_span(name: str, attributes: Mapping~str, object~) InMemorySpan
+reset() None
}
class TracingHook {
-_active_spans: dict~int, Span~
-_lock: Lock
-_record_output: bool
-_tracer: Tracer
+__init__(tracer: Tracer, record_output: bool)
+__call__(event: ExecEvent) None
-_handle_start(event: ExecEvent) None
-_handle_output(event: ExecEvent) None
-_handle_exit(event: ExecEvent) None
+_build_attributes(event: ExecEvent) dict~str, object~
}
class tracing_hook {
+tracing_hook(tracer: Tracer, record_output: bool) ExecHook
}
Span <|.. InMemorySpan
Tracer <|.. InMemoryTracer
ExecHook <|.. TracingHook
Tracer <.. TracingHook
TracingHook o--> Span
TracingHook o--> Tracer
tracing_hook ..> TracingHook
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: cuprum/adapters/logging_adapter.py Comment on lines +36 to +100 def structured_logging_hook( # noqa: PLR0913
*,
logger: logging.Logger | None = None,
plan_level: int = logging.DEBUG,
start_level: int = logging.INFO,
output_level: int = logging.DEBUG,
exit_level: int = logging.INFO,
) -> ExecHook:
"""Create an observe hook that logs execution events with structured data.
Parameters
----------
logger:
Logger instance for event emission. Defaults to
``logging.getLogger("cuprum.exec")``.
plan_level:
Log level for ``plan`` events (intent to execute). Default DEBUG.
start_level:
Log level for ``start`` events (process spawned). Default INFO.
output_level:
Log level for ``stdout``/``stderr`` events. Default DEBUG.
exit_level:
Log level for ``exit`` events (process completed). Default INFO.
Returns
-------
ExecHook
A hook suitable for use with ``sh.observe()``.
Notes
-----
This hook is synchronous and non-blocking. Log emission happens inline
with event processing. For high-throughput scenarios, consider using an
async handler or buffered logging configuration.
The hook attaches structured ``extra`` data to log records including:
- ``cuprum_phase``: Event phase (plan, start, stdout, stderr, exit)
- ``cuprum_program``: Program being executed
- ``cuprum_argv``: Full argument vector
- ``cuprum_pid``: Process ID (when available)
- ``cuprum_exit_code``: Exit code (for exit events)
- ``cuprum_duration_s``: Duration in seconds (for exit events)
- ``cuprum_tags``: Event tags as a dict
"""
log = logger or logging.getLogger(_DEFAULT_LOGGER_NAME)
levels: dict[str, int] = {
"plan": plan_level,
"start": start_level,
"stdout": output_level,
"stderr": output_level,
"exit": exit_level,
}
def hook(event: ExecEvent) -> None:
level = levels.get(event.phase, logging.DEBUG)
if not log.isEnabledFor(level):
return
extra = _build_extra(event)
message = _format_message(event)
log.log(level, message, extra=extra)
return hook❌ New issue: Excess Number of Function Arguments |
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: cuprum/adapters/metrics_adapter.py Comment on lines +76 to +94 def inc_counter(
self,
name: str,
value: float,
labels: cabc.Mapping[str, str],
) -> None:
"""Increment a counter metric.
Parameters
----------
name:
Metric name (e.g., ``cuprum_executions_total``).
value:
Amount to increment (usually 1.0).
labels:
Label key-value pairs for metric dimensions.
"""
...❌ New issue: Code Duplication |
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: cuprum/unittests/test_adapters.py Comment on lines +272 to +286 def test_sets_exit_attributes(self) -> None:
"""Hook sets exit_code and duration_s on span."""
builder, catalogue = _python_builder(project_name="tracing-exit")
cmd = builder("-c", "print('done')")
tracer = InMemoryTracer()
hook = TracingHook(tracer)
with scoped(allowlist=catalogue.allowlist), sh.observe(hook):
cmd.run_sync()
span = tracer.spans[0]
assert span.attributes.get("cuprum.exit_code") == 0
assert span.attributes.get("cuprum.duration_s") is not None
assert span.status_ok is True❌ New issue: Code Duplication |
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: cuprum/unittests/test_adapters.py Comment on lines +48 to +77 def test_logs_all_phases(self, caplog: pytest.LogCaptureFixture) -> None:
"""Hook logs plan, start, stdout, stderr, and exit events."""
builder, catalogue = _python_builder(project_name="logging-test")
cmd = builder(
"-c",
"\n".join(
(
"import sys",
"print('out1')",
"print('err1', file=sys.stderr)",
),
),
)
logger = logging.getLogger("cuprum.exec.test")
logger.setLevel(logging.DEBUG)
hook = structured_logging_hook(logger=logger)
with caplog.at_level(logging.DEBUG, logger="cuprum.exec.test"):
with scoped(allowlist=catalogue.allowlist), sh.observe(hook):
result = cmd.run_sync()
assert result.exit_code == 0
messages = [r.message for r in caplog.records]
assert any("cuprum.plan" in m for m in messages)
assert any("cuprum.start" in m for m in messages)
assert any("cuprum.stdout" in m and "out1" in m for m in messages)
assert any("cuprum.stderr" in m and "err1" in m for m in messages)
assert any("cuprum.exit" in m for m in messages)❌ New issue: Complex Method |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…with LogLevels dataclass - Introduced LogLevels dataclass to encapsulate log level configuration for phases. - Updated structured_logging_hook to accept LogLevels instance instead of separate level params. - Replaced direct dict with LogLevels instance in hook implementation. - Improved test suite to verify log level configuration using LogLevels. - Added helper assertions to improve test readability in test_adapters.py. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
cuprum/adapters/__init__.pycuprum/adapters/logging_adapter.pycuprum/adapters/metrics_adapter.pycuprum/adapters/tracing_adapter.pycuprum/unittests/test_adapters.pydocs/cuprum-design.mddocs/roadmap.mddocs/users-guide.mdtests/behaviour/test_telemetry_adapters.pytests/features/telemetry_adapters.feature
🧰 Additional context used
📓 Path-based instructions (10)
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/**/*.md: Use markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in thedocs/directory to reflect the latest state.
All documentation must adhere to the documentation style guide atdocs/documentation-style-guide.md.
Record any design decisions made in the relevant design document.
Files:
docs/users-guide.mddocs/roadmap.mddocs/cuprum-design.md
docs/users-guide.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/users-guide.md: Ensure new functionality is clearly documented in thedocs/users-guide.mdfile.
Ensure revised functionality is clearly documented in thedocs/users-guide.mdfile.
docs/users-guide.md: Document the builder pattern forsh.makeindocs/users-guide.md
Provide a scaffold and guidance for project-specific builders indocs/users-guide.md, including a template module and checklist
Files:
docs/users-guide.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: For Markdown files (.md only), ensure linting passes by runningmake markdownlint.
For Markdown files, validate Mermaid diagrams by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Files:
docs/users-guide.mddocs/roadmap.mddocs/cuprum-design.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/users-guide.mddocs/roadmap.mddocs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt}
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
docs/**/*.{md,mdx,rst,txt}: Use British English based on Oxford English Dictionary (en-GB-oxendict) conventions: use -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse, catalyse), -our suffixes (colour, behaviour, neighbour), -re suffixes (calibre, centre, fibre), double 'l' (cancelled, counsellor, cruellest), maintain 'e' (likeable, liveable, rateable), -ogue suffixes (analogue, catalogue)
The word 'outwith' is acceptable in documentation
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation, for example 'Lille Industries are expanding'
Write headings in sentence case in documentation
Use Markdown headings (#,##,###, and so on) in order without skipping levels
Always provide a language identifier for fenced code blocks in documentation; use 'plaintext' for non-code text
Use-as the first level bullet and renumber lists when items change in documentation
Prefer inline links using[text](url)or angle brackets around the URL in documentation
Ensure blank lines before and after bulleted lists and fenced blocks in documentation
Ensure tables have a delimiter line below the header row in documentation
Expand any uncommon acronym on first use in documentation, for example 'Continuous Integration (CI)'
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with[^label]in documentation
Include Mermaid diagrams in documentation where they add clarity
When embedding figures in documentation, useand provide brief alt text describing the content
Add a short description before each Mermaid diagram in documentation so screen readers can understand it
Files:
docs/users-guide.mddocs/roadmap.mddocs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt,rs}
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
Keep US spelling when used in API contexts, for example 'color'
Files:
docs/users-guide.mddocs/roadmap.mddocs/cuprum-design.md
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
Follow markdownlint recommendations for Markdown formatting
Files:
docs/users-guide.mddocs/roadmap.mddocs/cuprum-design.md
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: For Python files, ensure testing passes all relevant unit and behavioral tests by runningmake test.
For Python files, ensure linting passes by runningmake lint.
For Python files, ensure formatting adheres to standards by runningmake check-fmtand applyingmake fmtif needed.
For Python files, ensure type checking passes by runningmake typecheck.
For Python development, refer to Python-specific guidelines in the.rules/directory, including Python Code Style Guidelines, Context Managers, Exceptions and Logging, Generators, Project Configuration, Return Patterns, and Typing.
**/*.py: Use snake_case for Python filenames (e.g., http_client.py, task_queue.py)
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix private/internal functions and variables with a single underscore (_)
Enable full static type coverage using Pyright and maintain typing throughout the codebase
Use TypedDict or Dataclass for structured data, preferring @DataClass(slots=True) for internal-only usage
Avoid using Any type; use Unknown, generics, or cast() with documentation instead
Provide explicit return type annotations (e.g., -> None, -> str) for all public functions and class methods
Enforce strict mode in Pyright and treat all Pyright warnings as CI errors; use # pyright: ignore sparingly with explanation
Avoid side effects at import time; modules should not modify global state or perform actions on import
Use .env or settings modules for environment-specific configuration; never hardcode secrets
Use Ruff for formatting; let Ruff handle whitespace and formatting entirely
Use NumPy-format docstrings for public functions, classes, and modules
Use inline comments to explain tricky or non-obvious code logic and decisions
**/*.py: Use context managers (withcontextlib.contextmanageror class-based__enter__/__exit__) to encapsulate setup and teardown logic for resource management (f...
Files:
cuprum/adapters/__init__.pycuprum/adapters/metrics_adapter.pycuprum/unittests/test_adapters.pycuprum/adapters/logging_adapter.pycuprum/adapters/tracing_adapter.pytests/behaviour/test_telemetry_adapters.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep C90 / mccabe complexity ≤ 9
- Follow single responsibility and CQRS (command/query segregation)
- Prefer structural pattern matching to
- Prefer structural pattern matching over
isinstance()or imperative decomposition.- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.- All path manipulation must be performed using pathlib for cross platform safety. Do not use string manipulation for extraction of path components or filename ele...
Files:
cuprum/adapters/__init__.pycuprum/adapters/metrics_adapter.pycuprum/unittests/test_adapters.pycuprum/adapters/logging_adapter.pycuprum/adapters/tracing_adapter.pytests/behaviour/test_telemetry_adapters.py
**/unittests/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate unit tests with code using an unittests subdirectory with test_ prefix (e.g., user_auth/unittests/test_models.py)
Files:
cuprum/unittests/test_adapters.py
**/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
**/test_*.py: Use pytest idioms: prefer fixtures over setup/teardown methods, parametrize broadly, avoid unnecessary mocks
Group related tests using class with method names prefixed by test_
Write tests from a user's perspective; test public behavior rather than internals
Avoid mocking too much; prefer test doubles only for external services or non-deterministic behaviors
Files:
cuprum/unittests/test_adapters.pytests/behaviour/test_telemetry_adapters.py
**/*test*.py
📄 CodeRabbit inference engine (.rules/python-exception-design-raising-handling-and-logging.md)
Use specific exception types and message constraints with
pytest.raises(SpecificError, match=r"pattern")in tests; avoid overly broad exception assertions (B017)
Files:
cuprum/unittests/test_adapters.pytests/behaviour/test_telemetry_adapters.py
🧬 Code graph analysis (4)
cuprum/adapters/metrics_adapter.py (1)
cuprum/events.py (1)
ExecEvent(23-66)
cuprum/adapters/logging_adapter.py (1)
cuprum/events.py (1)
ExecEvent(23-66)
cuprum/adapters/tracing_adapter.py (1)
cuprum/events.py (1)
ExecEvent(23-66)
tests/behaviour/test_telemetry_adapters.py (7)
cuprum/adapters/logging_adapter.py (2)
structured_logging_hook(60-118)hook(109-116)cuprum/adapters/metrics_adapter.py (2)
InMemoryMetrics(118-163)MetricsHook(166-248)cuprum/adapters/tracing_adapter.py (2)
InMemoryTracer(190-223)TracingHook(226-355)cuprum/context.py (2)
scoped(251-287)observe(476-491)tests/helpers/catalogue.py (1)
python_catalogue(17-26)cuprum/sh.py (2)
make(529-545)builder(541-543)cuprum/catalogue.py (1)
allowlist(67-69)
🪛 LanguageTool
docs/users-guide.md
[uncategorized] ~499-~499: Loose punctuation mark.
Context: ...fields to log records: - cuprum_phase: Event phase (plan, start, stdout, stder...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~543-~543: Loose punctuation mark.
Context: ...k collects: - cuprum_executions_total: Counter incremented on each command sta...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~608-~608: Loose punctuation mark.
Context: ...th these attributes: - cuprum.program: The program being executed - `cuprum.ar...
(UNLIKELY_OPENING_PUNCTUATION)
docs/cuprum-design.md
[uncategorized] ~1035-~1035: Possible missing comma found.
Context: ...Protocol implementations must be thread-safe since hooks may be invoked from multi...
(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~1044-~1044: This word is normally spelled as one.
Context: ...lude program and project labels for multi-dimensional analysis. **Tracing adapter specific...
(EN_COMPOUNDS_MULTI_DIMENSIONAL)
🔍 Remote MCP
Let me search the web for information about Cuprum and its architecture:Based on my search, the web results about OpenTelemetry and general observability patterns are helpful but don't provide specific information about Cuprum's architecture. Let me search more specifically for Cuprum's GitHub repository and information about its command execution framework:Based on my research, I found information about general telemetry patterns and Prometheus metrics in Python, but the specific Cuprum Python framework mentioned in this PR appears to be a different project than what was found. Let me provide a summary of the relevant contextual information that would help review this PR.
Summary of Relevant Context for PR Review
Based on the information gathered, here is relevant context for effectively reviewing this telemetry adapters PR:
OpenTelemetry Standards Context
OpenTelemetry is an observability framework – an API, SDK, and tools that are designed to aid in the generation and collection of application telemetry data such as metrics, logs, and traces. The PR's approach of providing protocol-based adapters for logging, metrics, and tracing aligns with industry-standard observability patterns.
Prometheus Metrics Best Practices
The metrics adapter implementation follows established Prometheus conventions:
- A Counter in Prometheus is a cumulative metric representing a single numerical value that only increases. It is typically used to count events such as the number of requests served, tasks completed, or errors occurred.
- Histograms analyze data distributions in buckets.
The PR's MetricsCollector protocol and InMemoryMetrics implementation provide a vendor-neutral abstraction that allows connection to Prometheus and other monitoring backends without direct dependency coupling.
Decoupled Backend Patterns
The PR's use of Protocol/interface-based patterns is a recognized best practice: Libraries that produce telemetry data should only depend on opentelemetry-api, and defer the choice of the SDK to the application developer. Applications may depend on opentelemetry-sdk or another package that implements the API.
Code Quality Improvements Addressed
The PR comments show attention to CodeScene findings:
- The
structured_logging_hooksignature was refactored from 5 parameters to 2 using aLogLevelsdataclass—reducing function complexity. - Test code duplication was addressed by extracting helper methods and shared setup.
- The protocol-based design avoids unnecessary method consolidation while maintaining clarity and type safety.
Test Coverage
The PR includes comprehensive test coverage at multiple levels (unit, behavioral, and feature tests using Gherkin), which validates:
- All execution phases (plan, start, stdout, stderr, exit)
- Success and failure paths
- Thread-safe operations
- Multi-stage pipeline scenarios
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: Sourcery review
🔇 Additional comments (37)
docs/roadmap.md (1)
57-58: Roadmap checkpoint marked as complete.The checkbox is correctly updated to reflect completion of the telemetry adapters work—three protocol-based adapter modules with non-blocking, optional semantics, comprehensive tests, and design documentation are now shipped.
cuprum/adapters/logging_adapter.py (7)
1-34: LGTM: Well-structured module preamble.The module docstring clearly explains the adapter's purpose and provides a usage example. The TYPE_CHECKING guard correctly defers imports of
ExecEventandExecHookto avoid runtime overhead.
37-58: LGTM: LogLevels dataclass addresses function argument complexity.The dataclass encapsulates phase-level configuration cleanly, reducing the hook's parameter count as per the refactoring mentioned in the PR objectives.
60-118: LGTM: Well-designed hook with early exit optimisation.The
isEnabledForcheck at Line 111 avoids unnecessary work when the log level is disabled. Thelevel_map.getfallback toDEBUGhandles unknown phases gracefully.
121-139: LGTM: Clean conditional field extraction.The helper correctly converts
PathtostrandMappingtodict, ensuring JSON-serialisable output. Optional fields are added only when present.
142-163: LGTM: Idiomatic match/case for phase formatting.All known phases are handled with a catch-all fallback for future-proofing. The duration formatting at Lines 155-157 correctly handles the
Nonecase.
202-210: LGTM: Robust JSON serialisation helper.The recursive handling of nested structures and the
str()fallback for unknown types ensures safe serialisation.
213-217: LGTM: Public API exports are complete and alphabetically sorted.cuprum/adapters/metrics_adapter.py (5)
1-67: LGTM: Well-documented module with clear examples.The module docstring provides comprehensive usage examples for both the in-memory implementation and
prometheus_clientintegration. The TYPE_CHECKING guard correctly defers runtime-only imports.
69-114: LGTM: Protocol methods correctly remain separate.As noted in the PR comments, keeping
inc_counterandobserve_histogramas distinct methods preserves semantic clarity and type safety. Consolidating them would obscure their different purposes.
117-163: LGTM: Thread-safe in-memory implementation.The
Lockcorrectly protects bothcountersandhistogramsmutations. Ignoring labels is a reasonable simplification for a test/example collector, and it's clearly documented.
166-248: LGTM: Clean metrics collection with match/case.The hook correctly emits counters on
start,stdout,stderr, and failure exit events, plus a histogram observation for duration. The static_extract_labelsmethod keeps label extraction centralised.
251-276: LGTM: Factory function and exports are complete.The
metrics_hookfactory maintains API consistency with the other adapters.cuprum/adapters/tracing_adapter.py (7)
1-73: LGTM: Comprehensive module documentation with OpenTelemetry integration example.The docstring provides both in-memory and OpenTelemetry usage patterns, making it easy for users to implement their own backends.
75-126: LGTM: Minimal Span protocol with clear semantics.The keyword-only
okparameter inset_statusprevents accidental positional usage. The interface mirrors OpenTelemetry conventions effectively.
128-155: LGTM: Tracer protocol with thread-safety documentation.The protocol clearly states that implementations must be thread-safe.
158-186: LGTM: Simple in-memory span for testing.The implementation correctly stores attributes and events. Note that individual span methods are not thread-safe, but this is acceptable for the testing use case where spans are typically accessed from a single context.
189-223: LGTM: Thread-safe tracer implementation.The
Lockcorrectly protects thespanslist during creation and reset. Copyingattributesto a new dict at Line 214 prevents external mutation.
226-355: LGTM: Well-structured TracingHook with thread-safe span management.The hook correctly manages span lifecycle using PID as the key. The lock-protected
_active_spansdict ensures thread-safe access. The default status ofok=Truewhenexit_codeisNoneat Line 332 is a reasonable choice for unknown exit states.
358-387: LGTM: Factory function and exports are complete.The
tracing_hookfactory correctly forwards therecord_outputparameter toTracingHook.docs/users-guide.md (4)
469-476: LGTM: Clear introduction to telemetry adapters.The section correctly introduces the optional, non-blocking, protocol-based nature of the adapters.
477-519: LGTM: Structured logging documentation is accurate and complete.The field list and code examples align with the implementation in
logging_adapter.py.
521-583: LGTM: Metrics adapter documentation with Prometheus integration example.The counter and histogram list matches the implementation. The
PrometheusMetricsexample demonstrates the protocol pattern effectively.
660-674: LGTM: Design principles provide clear guidance.The four principles effectively summarise the adapter design philosophy.
cuprum/adapters/__init__.py (1)
1-40: LGTM: Package initialiser with clear usage guidance.The empty
__all__list is intentional, directing users to import from specific submodules. The docstring examples demonstrate the correct import patterns.docs/cuprum-design.md (1)
1008-1062: LGTM! Excellent documentation of telemetry adapter design.The telemetry adapter design decisions section is comprehensive, well-structured, and aligns perfectly with the protocol-based, non-blocking approach described in the PR objectives. The documentation clearly articulates design principles for metrics, tracing, and structured logging adapters.
tests/features/telemetry_adapters.feature (1)
1-35: LGTM! Well-structured BDD feature file.The feature file provides comprehensive coverage of telemetry adapters with clear scenarios for structured logging, metrics collection, and tracing. The scenarios appropriately cover both success and failure paths.
cuprum/unittests/test_adapters.py (6)
33-44: LGTM! Well-designed test helper.The
_python_builderhelper provides a clean, reusable way to construct test commands with configurable project names.
47-136: LGTM! Comprehensive structured logging tests.The test class thoroughly validates structured logging hook behaviour across all execution phases. The helper assertion methods (
_assert_phase_logged,_assert_output_logged) improve readability and reduce duplication, aligning with the refactoring mentioned in the PR objectives.
138-166: LGTM! Focused JSON formatter test.The test validates JSON formatting output with appropriate assertions for structured fields.
169-263: LGTM! Comprehensive metrics hook tests.The test class provides thorough coverage of metrics collection, including counters, histograms, failure tracking, and the factory function. The use of InMemoryMetrics for testing is appropriate.
268-295: LGTM! Excellent helper extraction.The
_run_traced_commandhelper successfully centralizes duplicated setup code across tracing tests, as described in the PR objectives. The NumPy-style docstring provides clear documentation.
265-435: LGTM! Comprehensive tracing hook tests.The test class thoroughly validates tracing functionality, including span creation, attributes, output recording, error status, pipeline support, and the factory function. The tests make effective use of the extracted helper method.
tests/behaviour/test_telemetry_adapters.py (4)
19-56: LGTM! Clear scenario mappings.The scenario functions provide clear links between the BDD feature file and test implementations.
59-121: LGTM! Well-structured test fixtures.The given fixtures provide appropriate setup for behavioural scenarios. The RecordCapture handler in
given_structured_logging_hookis a clean solution for capturing log records during tests.
123-219: LGTM! Effective when fixtures.The when fixtures appropriately execute commands for different scenarios. The
when_run_failurefixture cleverly handles both metrics and tracer hooks usingrequest.fixturenames.
221-310: LGTM! Comprehensive then assertions.The then fixtures provide thorough validation of telemetry adapter behaviour, with clear assertions and appropriate error messages.
This commit adds comprehensive unit tests for MetricsHook and TracingHook in telemetry adapters. Tests cover label passing, event handling without PID, and pipeline attribute setting on spans to improve coverage and reliability. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
cuprum/adapters/logging_adapter.pycuprum/adapters/tracing_adapter.pycuprum/unittests/test_adapters.pydocs/cuprum-design.mddocs/users-guide.mdtests/behaviour/test_telemetry_adapters.pytests/features/telemetry_adapters.feature
🧰 Additional context used
📓 Path-based instructions (10)
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/**/*.md: Use markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in thedocs/directory to reflect the latest state.
All documentation must adhere to the documentation style guide atdocs/documentation-style-guide.md.
Record any design decisions made in the relevant design document.
Files:
docs/users-guide.mddocs/cuprum-design.md
docs/users-guide.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/users-guide.md: Ensure new functionality is clearly documented in thedocs/users-guide.mdfile.
Ensure revised functionality is clearly documented in thedocs/users-guide.mdfile.
docs/users-guide.md: Document the builder pattern forsh.makeindocs/users-guide.md
Provide a scaffold and guidance for project-specific builders indocs/users-guide.md, including a template module and checklist
Files:
docs/users-guide.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: For Markdown files (.md only), ensure linting passes by runningmake markdownlint.
For Markdown files, validate Mermaid diagrams by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Files:
docs/users-guide.mddocs/cuprum-design.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/users-guide.mddocs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt}
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
docs/**/*.{md,mdx,rst,txt}: Use British English based on Oxford English Dictionary (en-GB-oxendict) conventions: use -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse, catalyse), -our suffixes (colour, behaviour, neighbour), -re suffixes (calibre, centre, fibre), double 'l' (cancelled, counsellor, cruellest), maintain 'e' (likeable, liveable, rateable), -ogue suffixes (analogue, catalogue)
The word 'outwith' is acceptable in documentation
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation, for example 'Lille Industries are expanding'
Write headings in sentence case in documentation
Use Markdown headings (#,##,###, and so on) in order without skipping levels
Always provide a language identifier for fenced code blocks in documentation; use 'plaintext' for non-code text
Use-as the first level bullet and renumber lists when items change in documentation
Prefer inline links using[text](url)or angle brackets around the URL in documentation
Ensure blank lines before and after bulleted lists and fenced blocks in documentation
Ensure tables have a delimiter line below the header row in documentation
Expand any uncommon acronym on first use in documentation, for example 'Continuous Integration (CI)'
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with[^label]in documentation
Include Mermaid diagrams in documentation where they add clarity
When embedding figures in documentation, useand provide brief alt text describing the content
Add a short description before each Mermaid diagram in documentation so screen readers can understand it
Files:
docs/users-guide.mddocs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt,rs}
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
Keep US spelling when used in API contexts, for example 'color'
Files:
docs/users-guide.mddocs/cuprum-design.md
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
Follow markdownlint recommendations for Markdown formatting
Files:
docs/users-guide.mddocs/cuprum-design.md
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: For Python files, ensure testing passes all relevant unit and behavioral tests by runningmake test.
For Python files, ensure linting passes by runningmake lint.
For Python files, ensure formatting adheres to standards by runningmake check-fmtand applyingmake fmtif needed.
For Python files, ensure type checking passes by runningmake typecheck.
For Python development, refer to Python-specific guidelines in the.rules/directory, including Python Code Style Guidelines, Context Managers, Exceptions and Logging, Generators, Project Configuration, Return Patterns, and Typing.
**/*.py: Use snake_case for Python filenames (e.g., http_client.py, task_queue.py)
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix private/internal functions and variables with a single underscore (_)
Enable full static type coverage using Pyright and maintain typing throughout the codebase
Use TypedDict or Dataclass for structured data, preferring @DataClass(slots=True) for internal-only usage
Avoid using Any type; use Unknown, generics, or cast() with documentation instead
Provide explicit return type annotations (e.g., -> None, -> str) for all public functions and class methods
Enforce strict mode in Pyright and treat all Pyright warnings as CI errors; use # pyright: ignore sparingly with explanation
Avoid side effects at import time; modules should not modify global state or perform actions on import
Use .env or settings modules for environment-specific configuration; never hardcode secrets
Use Ruff for formatting; let Ruff handle whitespace and formatting entirely
Use NumPy-format docstrings for public functions, classes, and modules
Use inline comments to explain tricky or non-obvious code logic and decisions
**/*.py: Use context managers (withcontextlib.contextmanageror class-based__enter__/__exit__) to encapsulate setup and teardown logic for resource management (f...
Files:
cuprum/adapters/logging_adapter.pycuprum/adapters/tracing_adapter.pycuprum/unittests/test_adapters.pytests/behaviour/test_telemetry_adapters.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep C90 / mccabe complexity ≤ 9
- Follow single responsibility and CQRS (command/query segregation)
- Prefer structural pattern matching to
- Prefer structural pattern matching over
isinstance()or imperative decomposition.- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.- All path manipulation must be performed using pathlib for cross platform safety. Do not use string manipulation for extraction of path components or filename ele...
Files:
cuprum/adapters/logging_adapter.pycuprum/adapters/tracing_adapter.pycuprum/unittests/test_adapters.pytests/behaviour/test_telemetry_adapters.py
**/unittests/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate unit tests with code using an unittests subdirectory with test_ prefix (e.g., user_auth/unittests/test_models.py)
Files:
cuprum/unittests/test_adapters.py
**/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
**/test_*.py: Use pytest idioms: prefer fixtures over setup/teardown methods, parametrize broadly, avoid unnecessary mocks
Group related tests using class with method names prefixed by test_
Write tests from a user's perspective; test public behavior rather than internals
Avoid mocking too much; prefer test doubles only for external services or non-deterministic behaviors
Files:
cuprum/unittests/test_adapters.pytests/behaviour/test_telemetry_adapters.py
**/*test*.py
📄 CodeRabbit inference engine (.rules/python-exception-design-raising-handling-and-logging.md)
Use specific exception types and message constraints with
pytest.raises(SpecificError, match=r"pattern")in tests; avoid overly broad exception assertions (B017)
Files:
cuprum/unittests/test_adapters.pytests/behaviour/test_telemetry_adapters.py
🧬 Code graph analysis (3)
cuprum/adapters/logging_adapter.py (1)
cuprum/events.py (1)
ExecEvent(23-66)
cuprum/adapters/tracing_adapter.py (1)
cuprum/events.py (1)
ExecEvent(23-66)
cuprum/unittests/test_adapters.py (6)
cuprum/adapters/logging_adapter.py (4)
LogLevels(39-58)structured_logging_hook(61-119)hook(110-117)format(185-198)cuprum/adapters/metrics_adapter.py (8)
InMemoryMetrics(118-163)MetricsHook(166-248)metrics_hook(251-268)inc_counter(76-94)inc_counter(137-145)observe_histogram(96-114)observe_histogram(147-157)reset(159-163)cuprum/adapters/tracing_adapter.py (9)
InMemorySpan(159-186)InMemoryTracer(190-223)TracingHook(226-355)tracing_hook(358-377)reset(220-223)start_span(135-155)start_span(206-218)end(123-125)end(184-186)cuprum/catalogue.py (3)
ProgramCatalogue(56-119)ProjectSettings(30-40)allowlist(67-69)cuprum/context.py (2)
scoped(251-287)observe(476-491)cuprum/events.py (1)
ExecEvent(23-66)
🪛 LanguageTool
docs/users-guide.md
[uncategorized] ~499-~499: Loose punctuation mark.
Context: ...fields to log records: - cuprum_phase: Event phase (plan, start, stdout, stder...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~524-~524: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...and histograms. It uses a protocol class so the backend can be implemented with any...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~543-~543: Loose punctuation mark.
Context: ...k collects: - cuprum_executions_total: Counter incremented on each command sta...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~608-~608: Loose punctuation mark.
Context: ...th these attributes: - cuprum.program: The program being executed - `cuprum.ar...
(UNLIKELY_OPENING_PUNCTUATION)
docs/cuprum-design.md
[uncategorized] ~1035-~1035: Possible missing comma found.
Context: ...Protocol implementations must be thread-safe since hooks may be invoked from multi...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (11)
cuprum/adapters/logging_adapter.py (1)
1-216: LGTM!The structured logging adapter is well-designed with clear separation of concerns. The
LogLevelsdataclass properly encapsulates configuration, the hook factory returns a clean closure, and theJsonLoggingFormatterprovides structured output for log aggregation. The previous feedback about movingimport jsonto module level has been addressed.cuprum/adapters/tracing_adapter.py (1)
274-335: LGTM — TracingHook implementation is thread-safe and well-structured.The span lifecycle management using PID as the correlation key is correct. The lock usage ensures thread safety for concurrent command executions. The pattern matching for phase dispatch is clean and idiomatic.
docs/users-guide.md (1)
469-674: LGTM — comprehensive telemetry documentation.The documentation clearly explains each adapter with practical code examples. The design principles section provides valuable guidance for users implementing custom backends. Previous feedback about the
set_statussignature has been addressed (line 637 now showsdef set_status(self, *, ok):).docs/cuprum-design.md (1)
1008-1062: LGTM — design decisions are clearly documented.The telemetry adapter design section provides excellent rationale for protocol-based decoupling, non-blocking execution, and thread safety requirements. This aligns well with the implementation in the adapter modules.
tests/features/telemetry_adapters.feature (1)
1-35: LGTM — well-structured feature scenarios.The Gherkin scenarios provide good behavioural coverage for telemetry adapters: structured logging, metrics collection (success and failure paths), and tracing (span creation and error status). The background step correctly establishes shared fixtures.
cuprum/unittests/test_adapters.py (4)
47-61: Good helper extraction for assertion methods.The
_assert_phase_loggedand_assert_output_loggedstatic methods improve readability and reduce code duplication in the test class. This addresses previous feedback about complex test methods.
264-316: Label verification test properly validates metrics labelling.The
LabelRecordingCollectortest double correctly captures metric calls with their labels, verifying thatprogramandprojectlabels are passed through to the collector. This addresses the previous feedback about testing label propagation.
490-600: Edge case coverage for pid-less events and pipeline attributes.The tests for
pid=Noneevents (line 490) and pipeline tag attributes (line 549) provide important coverage for edge cases in the tracing hook. These address previous feedback about testing implemented behaviours that weren't covered.
321-348: Helper method reduces test boilerplate effectively.The
_run_traced_commandhelper centralises builder/catalogue setup, tracer/hook creation, and execution, returning the tracer and span for assertions. This addresses the previous feedback about duplicate setup across test methods.tests/behaviour/test_telemetry_adapters.py (2)
88-102: RecordCapture handler is a clean solution for log capture.The inline
RecordCapturehandler class properly extendslogging.Handlerand captures records for assertion. This approach avoids coupling to pytest's caplog fixture within BDD step definitions.
168-192: Dynamic hook selection based on fixture availability.The
when_run_failurestep cleverly determines which hook to use by checkingrequest.fixturenames. This allows the same step definition to work with both metrics and tracer scenarios without duplication.
…ation Added slots=True to the dataclass decorators for InMemorySpan and InMemoryTracer classes in tracing_adapter.py to optimize memory usage and attribute access. Additionally, expanded the docstring in test_telemetry_adapters.py to clarify the usage of typ.cast in test code for improved type safety understanding. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
cuprum/adapters/tracing_adapter.pytests/behaviour/test_telemetry_adapters.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: For Python files, ensure testing passes all relevant unit and behavioral tests by runningmake test.
For Python files, ensure linting passes by runningmake lint.
For Python files, ensure formatting adheres to standards by runningmake check-fmtand applyingmake fmtif needed.
For Python files, ensure type checking passes by runningmake typecheck.
For Python development, refer to Python-specific guidelines in the.rules/directory, including Python Code Style Guidelines, Context Managers, Exceptions and Logging, Generators, Project Configuration, Return Patterns, and Typing.
**/*.py: Use snake_case for Python filenames (e.g., http_client.py, task_queue.py)
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix private/internal functions and variables with a single underscore (_)
Enable full static type coverage using Pyright and maintain typing throughout the codebase
Use TypedDict or Dataclass for structured data, preferring @DataClass(slots=True) for internal-only usage
Avoid using Any type; use Unknown, generics, or cast() with documentation instead
Provide explicit return type annotations (e.g., -> None, -> str) for all public functions and class methods
Enforce strict mode in Pyright and treat all Pyright warnings as CI errors; use # pyright: ignore sparingly with explanation
Avoid side effects at import time; modules should not modify global state or perform actions on import
Use .env or settings modules for environment-specific configuration; never hardcode secrets
Use Ruff for formatting; let Ruff handle whitespace and formatting entirely
Use NumPy-format docstrings for public functions, classes, and modules
Use inline comments to explain tricky or non-obvious code logic and decisions
**/*.py: Use context managers (withcontextlib.contextmanageror class-based__enter__/__exit__) to encapsulate setup and teardown logic for resource management (f...
Files:
cuprum/adapters/tracing_adapter.pytests/behaviour/test_telemetry_adapters.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep C90 / mccabe complexity ≤ 9
- Follow single responsibility and CQRS (command/query segregation)
- Prefer structural pattern matching to
- Prefer structural pattern matching over
isinstance()or imperative decomposition.- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.- All path manipulation must be performed using pathlib for cross platform safety. Do not use string manipulation for extraction of path components or filename ele...
Files:
cuprum/adapters/tracing_adapter.pytests/behaviour/test_telemetry_adapters.py
**/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
**/test_*.py: Use pytest idioms: prefer fixtures over setup/teardown methods, parametrize broadly, avoid unnecessary mocks
Group related tests using class with method names prefixed by test_
Write tests from a user's perspective; test public behavior rather than internals
Avoid mocking too much; prefer test doubles only for external services or non-deterministic behaviors
Files:
tests/behaviour/test_telemetry_adapters.py
**/*test*.py
📄 CodeRabbit inference engine (.rules/python-exception-design-raising-handling-and-logging.md)
Use specific exception types and message constraints with
pytest.raises(SpecificError, match=r"pattern")in tests; avoid overly broad exception assertions (B017)
Files:
tests/behaviour/test_telemetry_adapters.py
🧬 Code graph analysis (1)
cuprum/adapters/tracing_adapter.py (2)
cuprum/events.py (1)
ExecEvent(23-66)cuprum/adapters/metrics_adapter.py (1)
reset(159-163)
🔍 Remote MCP
Let me try a different approach and search for information about telemetry and observability patterns in Python projects:
Summary of Relevant Context for PR #17 Review
Based on web research into telemetry best practices, here is the key context that validates the design decisions in this PR:
OpenTelemetry Tracing Patterns
The tracing adapter's design aligns with OpenTelemetry's span pattern: setting attributes on spans, recording events as human-readable messages, and setting span status based on operation outcomes. A span represents a single operation within a trace, which matches Cuprum's use case of tracking execution events. OpenTelemetry span processors are designed to run synchronously on the thread that starts/ends spans without blocking or throwing exceptions, supporting the PR's non-blocking design pattern.
Prometheus Metrics & Thread Safety
Prometheus client libraries must be thread-safe, requiring concurrency control with consideration for multi-core performance. The PR's InMemoryMetrics dataclass with threading.Lock directly addresses this requirement. Prometheus client libraries presume a threaded model where metrics are shared across workers, validating the protocol-based collector approach that allows backend flexibility.
Structured Logging & JSON Formatting
JSON logging is a best practice for centralized log management; machines can easily parse and analyze this standard format, and JSON is easily customizable to include any attributes without updating log processing pipelines. The python-json-logger library supports adding custom fields via the add_fields method and extra dictionaries that get added at the root level of JSON entries, aligning with the PR's cuprum_* field prefixing approach for log aggregation. The cleanest structured logging implementation replaces standard logging.Formatter with JsonFormatter via dictConfig, automatically including standard fields as JSON keys.
Non-Blocking Hook Execution
OpenTelemetry span processors are registered and invoked in order, called synchronously on the thread starting/ending spans, and must not block or throw exceptions. This validates the PR's synchronous, non-blocking adapter hook design as a standard pattern in observability frameworks.
Design Validation
The PR's protocol-based approach (defining MetricsCollector, Span, Tracer as protocols with in-memory reference implementations) follows established patterns in OpenTelemetry and Prometheus for decoupling interface definitions from backend implementations, enabling users to plug in their preferred monitoring systems without library dependencies.
[::web_search::], [::Context7::]
…n telemetry tests Extracted command execution logic into helper _execute_python_command to reduce code duplication and improve clarity in telemetry adapter behaviour tests. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/behaviour/test_telemetry_adapters.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: For Python files, ensure testing passes all relevant unit and behavioral tests by runningmake test.
For Python files, ensure linting passes by runningmake lint.
For Python files, ensure formatting adheres to standards by runningmake check-fmtand applyingmake fmtif needed.
For Python files, ensure type checking passes by runningmake typecheck.
For Python development, refer to Python-specific guidelines in the.rules/directory, including Python Code Style Guidelines, Context Managers, Exceptions and Logging, Generators, Project Configuration, Return Patterns, and Typing.
**/*.py: Use snake_case for Python filenames (e.g., http_client.py, task_queue.py)
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix private/internal functions and variables with a single underscore (_)
Enable full static type coverage using Pyright and maintain typing throughout the codebase
Use TypedDict or Dataclass for structured data, preferring @DataClass(slots=True) for internal-only usage
Avoid using Any type; use Unknown, generics, or cast() with documentation instead
Provide explicit return type annotations (e.g., -> None, -> str) for all public functions and class methods
Enforce strict mode in Pyright and treat all Pyright warnings as CI errors; use # pyright: ignore sparingly with explanation
Avoid side effects at import time; modules should not modify global state or perform actions on import
Use .env or settings modules for environment-specific configuration; never hardcode secrets
Use Ruff for formatting; let Ruff handle whitespace and formatting entirely
Use NumPy-format docstrings for public functions, classes, and modules
Use inline comments to explain tricky or non-obvious code logic and decisions
**/*.py: Use context managers (withcontextlib.contextmanageror class-based__enter__/__exit__) to encapsulate setup and teardown logic for resource management (f...
Files:
tests/behaviour/test_telemetry_adapters.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep C90 / mccabe complexity ≤ 9
- Follow single responsibility and CQRS (command/query segregation)
- Prefer structural pattern matching to
- Prefer structural pattern matching over
isinstance()or imperative decomposition.- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.- All path manipulation must be performed using pathlib for cross platform safety. Do not use string manipulation for extraction of path components or filename ele...
Files:
tests/behaviour/test_telemetry_adapters.py
**/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
**/test_*.py: Use pytest idioms: prefer fixtures over setup/teardown methods, parametrize broadly, avoid unnecessary mocks
Group related tests using class with method names prefixed by test_
Write tests from a user's perspective; test public behavior rather than internals
Avoid mocking too much; prefer test doubles only for external services or non-deterministic behaviors
Files:
tests/behaviour/test_telemetry_adapters.py
**/*test*.py
📄 CodeRabbit inference engine (.rules/python-exception-design-raising-handling-and-logging.md)
Use specific exception types and message constraints with
pytest.raises(SpecificError, match=r"pattern")in tests; avoid overly broad exception assertions (B017)
Files:
tests/behaviour/test_telemetry_adapters.py
🧬 Code graph analysis (1)
tests/behaviour/test_telemetry_adapters.py (7)
cuprum/adapters/logging_adapter.py (2)
structured_logging_hook(61-119)hook(110-117)cuprum/adapters/metrics_adapter.py (2)
InMemoryMetrics(118-163)MetricsHook(166-248)cuprum/adapters/tracing_adapter.py (2)
InMemoryTracer(190-223)TracingHook(226-355)cuprum/context.py (2)
scoped(251-287)observe(476-491)tests/helpers/catalogue.py (1)
python_catalogue(17-26)cuprum/sh.py (2)
make(529-545)builder(541-543)cuprum/catalogue.py (1)
allowlist(67-69)
🔇 Additional comments (2)
tests/behaviour/test_telemetry_adapters.py (2)
1-30: LGTM: Module docstring clearly documents typing trade-offs.The module docstring appropriately explains the extensive use of
typ.cast()with pytest-bdd fixtures, addressing the previous review concern about type safety. This documentation helps future maintainers understand the trade-off.
136-224: LGTM: Helper function eliminates duplication effectively.The
_execute_python_commandhelper successfully centralises the execution pattern across allwhen_*fixtures, addressing the previous review comment. The refactor maintains pytest-bdd semantics whilst improving maintainability.
Add an explicit assertion to check that a start record exists in the telemetry log records before accessing its attributes. This prevents potential exceptions if the record is missing, improving test stability and clarity. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
… runs Refactor telemetry adapter tests and feature files to distinctly handle failing commands with metrics tracking and tracing. Split the previous single "run a command that fails" step into two separate steps: one for metrics and one for tracing. This improves clarity and explicitness in test behaviour and scenarios. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: tests/behaviour/test_telemetry_adapters.py Comment on lines +159 to +173 def when_run_stdout_stderr(
behaviour_state: dict[str, object],
python_cmd_fixture: dict[str, object],
logging_hook_fixture: dict[str, object],
) -> None:
"""Run a command that writes to both streams."""
hook = typ.cast("ExecHook", logging_hook_fixture["hook"])
script = "\n".join(
(
"import sys",
"print('stdout-line')",
"print('stderr-line', file=sys.stderr)",
),
)
_execute_python_command(behaviour_state, python_cmd_fixture, hook, script)❌ New issue: Code Duplication |
This comment was marked as resolved.
This comment was marked as resolved.
…ooks Refactored telemetry adapter tests by introducing a helper function `_run_command_with_hook` to replace repetitive code when running python commands with hooks. This improves code readability and reduces duplication in test steps. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
leynos
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/behaviour/test_telemetry_adapters.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: For Python files, ensure testing passes all relevant unit and behavioral tests by runningmake test.
For Python files, ensure linting passes by runningmake lint.
For Python files, ensure formatting adheres to standards by runningmake check-fmtand applyingmake fmtif needed.
For Python files, ensure type checking passes by runningmake typecheck.
For Python development, refer to Python-specific guidelines in the.rules/directory, including Python Code Style Guidelines, Context Managers, Exceptions and Logging, Generators, Project Configuration, Return Patterns, and Typing.
**/*.py: Use snake_case for Python filenames (e.g., http_client.py, task_queue.py)
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix private/internal functions and variables with a single underscore (_)
Enable full static type coverage using Pyright and maintain typing throughout the codebase
Use TypedDict or Dataclass for structured data, preferring @DataClass(slots=True) for internal-only usage
Avoid using Any type; use Unknown, generics, or cast() with documentation instead
Provide explicit return type annotations (e.g., -> None, -> str) for all public functions and class methods
Enforce strict mode in Pyright and treat all Pyright warnings as CI errors; use # pyright: ignore sparingly with explanation
Avoid side effects at import time; modules should not modify global state or perform actions on import
Use .env or settings modules for environment-specific configuration; never hardcode secrets
Use Ruff for formatting; let Ruff handle whitespace and formatting entirely
Use NumPy-format docstrings for public functions, classes, and modules
Use inline comments to explain tricky or non-obvious code logic and decisions
**/*.py: Use context managers (withcontextlib.contextmanageror class-based__enter__/__exit__) to encapsulate setup and teardown logic for resource management (f...
Files:
tests/behaviour/test_telemetry_adapters.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep C90 / mccabe complexity ≤ 9
- Follow single responsibility and CQRS (command/query segregation)
- Prefer structural pattern matching to
- Prefer structural pattern matching over
isinstance()or imperative decomposition.- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.- All path manipulation must be performed using pathlib for cross platform safety. Do not use string manipulation for extraction of path components or filename ele...
Files:
tests/behaviour/test_telemetry_adapters.py
**/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
**/test_*.py: Use pytest idioms: prefer fixtures over setup/teardown methods, parametrize broadly, avoid unnecessary mocks
Group related tests using class with method names prefixed by test_
Write tests from a user's perspective; test public behavior rather than internals
Avoid mocking too much; prefer test doubles only for external services or non-deterministic behaviors
Files:
tests/behaviour/test_telemetry_adapters.py
**/*test*.py
📄 CodeRabbit inference engine (.rules/python-exception-design-raising-handling-and-logging.md)
Use specific exception types and message constraints with
pytest.raises(SpecificError, match=r"pattern")in tests; avoid overly broad exception assertions (B017)
Files:
tests/behaviour/test_telemetry_adapters.py
🧬 Code graph analysis (1)
tests/behaviour/test_telemetry_adapters.py (7)
cuprum/adapters/logging_adapter.py (2)
structured_logging_hook(61-119)hook(110-117)cuprum/adapters/metrics_adapter.py (2)
InMemoryMetrics(118-163)MetricsHook(166-248)cuprum/adapters/tracing_adapter.py (2)
InMemoryTracer(190-223)TracingHook(226-355)cuprum/context.py (2)
scoped(251-287)observe(476-491)tests/helpers/catalogue.py (1)
python_catalogue(17-26)cuprum/sh.py (2)
make(529-545)builder(541-543)cuprum/catalogue.py (1)
allowlist(67-69)
🔍 Remote MCP
Since the Deepwiki tool doesn't have the Cuprum repository indexed, let me search for general information about the architecture and patterns used in this PR.Based on the search results for telemetry adapter patterns and thread-safety practices, I can now provide a focused summary of relevant context for reviewing this PR.
Summary of Additional Context for PR Review
Telemetry Adapter Design Patterns
OpenTelemetry uses a well-established architecture with API, SDK, plugin, and semantic conventions packages, developed by the Cloud Native Computing Foundation to provide a standardized, vendor-agnostic solution for telemetry collection. This PR's approach of using Protocol-based adapters aligns with established OpenTelemetry principles—adapters should depend on API definitions rather than SDK implementations to enable reusability across different signal types.
Thread-Safety Implementation Patterns
The PR's use of threading.Lock in InMemoryMetrics and InMemoryTracer follows established Python concurrency best practices. Thread safety refers to operations being performable by multiple threads concurrently without causing erroneous behavior, requiring proper synchronization mechanisms to ensure shared resources are accessed in a controlled manner. In Python, operations like incrementing variables may not be inherently atomic, requiring use of Lock or RLock to encapsulate non-atomic operations and maintain thread safety.
Signal-Type Separation Pattern
The PR implements separate adapters for logging, metrics, and tracing, which aligns with modern observability architecture. Specialized fleets deployed per telemetry signal type—one for traces, one for metrics, and one for logs—allows each gateway fleet to be configured and optimized only for its specific signal type. This supports the PR's non-blocking, optional design.
Backend-Agnostic Extensibility
OpenTelemetry is designed to be extensible through adding receivers to the Collector, loading custom instrumentation libraries, and creating new exporters for custom backends. The PR's Protocol-based design (MetricsCollector, Tracer, Span) follows this pattern, enabling users to implement custom backends without modifying the adapters themselves.
[::web_search::]
🔇 Additional comments (6)
tests/behaviour/test_telemetry_adapters.py (6)
1-30: Excellent documentation of type-casting constraints.The module docstring clearly explains why
typ.cast("typ.Any", ...)is necessary for pytest-bdd fixture values, making the trade-off explicit for future maintainers. The TYPE_CHECKING guard correctly isolates ExecHook to avoid runtime imports. This addresses the previous review concern about documenting the limitation.
31-47: Module-level script constants improve maintainability.Extracting the Python scripts to module-level constants with underscore-prefixed names eliminates duplication in the when step functions and makes the test scenarios easier to maintain. This successfully addresses the previous review recommendation.
50-88: Scenario declarations properly structured.All scenario declarations have explicit return type annotations and docstrings. The renamed scenario at line 52 ("Structured logging hook emits structured records") correctly reflects the actual behaviour being tested, addressing the previous review feedback about the JSON-focused naming mismatch.
90-151: Fixtures are well-structured with proper type annotations.All fixture functions have correct return type annotations and clear docstrings. The RecordCapture helper class at lines 119-125 includes explicit
-> Noneannotations on both__init__andemit, confirming this previous review concern has been addressed.
200-257: When steps properly leverage helpers and explicit fixture selection.All when step functions correctly delegate to
_run_command_with_hookwith module-level script constants, eliminating duplication. The split betweenwhen_run_failure_with_metricsandwhen_run_failure_with_tracerat lines 224-245 explicitly selects the appropriate fixture, addressing the previous review concern about fragile fixture inspection. Well done.
260-274: Phase logging assertions include helpful messages.All assertions in
then_all_phases_loggedinclude descriptive messages, making test failures immediately actionable.
Added descriptive assertion messages to improve test diagnostics in telemetry adapter behaviour tests. This enhancement facilitates easier debugging by providing clear failure explanations without changing test logic. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
1 participant
Summary
Changes
Adapters
Tests
Documentation
Why
How to use
Structured logging (example):
Metrics (example):
Tracing (example):
Testing
Impact
If you want additional adapters (e.g., OpenTelemetry integration examples) or more metrics/trace backends, we can extend the protocols and provide additional reference implementations in follow-up PRs.
🌿 Generated by Terry
ℹ️ Tag @terragon-labs to ask questions and address PR feedback
📎 Task: https://www.terragonlabs.com/task/69f5f0f8-22aa-4450-8e6b-c14ef28373bf
Summary by Sourcery
Add optional telemetry adapters for structured logging, metrics, and tracing over Cuprum execution events, with protocol-based abstractions and in-memory reference implementations.
New Features:
Enhancements:
Tests: