Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions .agents/skills/python-structured-logging/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@ description: Review or improve Python logging with structlog, stdlib logging, or

Make logs useful operational events while preserving the user's scope and project contracts.

## Workflow

1. Determine whether the request is a review, a local logging change, or integration work. Review requests produce findings without edits.
2. Trace the affected logging path: its API, formatter/processors, event consumers, and failure owner as relevant. For a local change, stop when you can identify where the changed event renders and which contracts it touches; record unavailable configuration as an assumption.
3. Read the matching resources below, perform the requested work, and verify the applicable completion criteria.

## Scope and contracts

- Review requests need findings with locations and concrete effects, not edits.
- Inspect the logger API, formatter/processors, event conventions, and failure ownership. Keep the stack and wrappers; migration requires authorization.
- Keep the stack and wrappers; migration requires authorization.
- Preserve business behavior, signatures, exception propagation, and retry decisions.
- Event names and fields may feed alerts, dashboards, and queries. Preserve existing contracts, including dotted names. Before an authorized rename, inspect available consumers and explain their updates; report external consumers you cannot verify.
- With no existing convention, use stable `snake_case` events such as `invoice_processed` and put variable values in fields. Follow existing field names; give new units explicit keys such as `duration_ms`.
Expand All @@ -26,7 +31,7 @@ Read the matching example pair for substantial refactoring, integration setup, o

Read only the relevant reference when:

- Changing formatters/processors, diagnosing missing fields, serialization, or duplicate handlers: [output pipeline](references/output-pipeline.md).
- Changing formatters/processors, configuring runtime logging, diagnosing missing fields, serialization, or duplicate handlers, or checking sensitive data across output paths: [output pipeline](references/output-pipeline.md).
- Working with cross-module or concurrent context or cleanup: [context lifecycle](references/context-lifecycle.md).
- Resolving failure ownership or sanitizing exception output: [exceptions and sensitive data](references/exceptions-and-sensitive-data.md).

Expand All @@ -40,10 +45,12 @@ Read only the relevant reference when:

## Verify

Use focused tests/output capture proportional to the change, through the complete configured runtime output boundary (see [output pipeline](references/output-pipeline.md)):
Choose checks for the affected behavior; combine criteria when a task spans branches:

- Contracts and application behavior preserved; fields survive formatting without API errors.
- One appropriate failure record, with traceback when needed; no sensitive values in fields, messages, or rendered exceptions.
- Request/job context isolated and cleaned up.
- **Review:** each finding has a location, concrete effect, and supporting evidence. Distinguish observed defects from unverified risks. Static review is sufficient when it establishes the finding; report runtime assumptions without requiring application startup.
- **Local change:** capture the changed event through the configured formatter, or a representative formatter when runtime configuration is unavailable. Confirm added fields survive and affected event contracts and application behavior remain intact. Keep checks scoped to the changed path.
- **Pipeline or sensitive-output change:** capture the complete configured runtime output boundary described in [output pipeline](references/output-pipeline.md), including failure and fallback paths. Verify configured output format and absence of sensitive values in fields, messages, and rendered exceptions.
- **Failure ownership change:** exercise the affected failure path; verify one appropriate failure record, traceback when needed, and unchanged propagation/retry behavior.
- **Context change:** exercise overlapping operations and cleanup on success and failure; check parent-context restoration for nested scopes.

Report checks performed and unverified pipeline assumptions.
Report checks performed, their observed results, and unverified pipeline assumptions. A representative formatter check does not establish full runtime safety.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ For a structlog application already using contextvars, check that `merge_context
Test two overlapping requests with distinct IDs and a subsequent operation with no ID. Their outputs must not share identifiers. Thread/task boundaries and hybrid sync/async frameworks may require explicit propagation; do not assume every execution context shares the same values.

For stdlib, retain the existing adapter, filter, or record factory. Check the supported Python version and adapter behavior before relying on per-call `extra` merging.

Source: [structlog context variables](https://www.structlog.org/en/stable/contextvars.html).
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

Choose the owner based on the call chain: a request boundary, worker, or command may already record failures. Adding another exception log below it can duplicate alerts. If a lower layer owns the only failure record and propagates the exception, document that its caller must not log it again.

`logger.exception` normally renders the exception message too. Allowlisting structured fields alone does not sanitize credentials embedded in a URL, exception text, or captured locals. Exercise the configured sanitizer with synthetic sensitive values and inspect its final output. Never use real secrets as fixtures.
`logger.exception` normally renders the exception message too. Allowlisting structured fields alone does not sanitize credentials embedded in a URL, exception text, or captured locals. Exercise the configured sanitizer with synthetic sensitive values and inspect its final output. Never use real secrets as fixtures. When changing sanitization or checking safety across output paths, follow [output pipeline](output-pipeline.md) to cover runtime loggers and formatter fallback as well as application records.

The paired examples use a non-retryable negative amount to illustrate preserved behavior. Both versions raise the same exception for the same input. The good version changes logging only; it does not add retry logic or invent payment metadata.
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ Capture the final rendered output as well as records. Check JSON decoding where
At the application's configuration boundary, include framework/server/access/error/lifecycle loggers and the actual stdout/stderr or collector path, not just the application logger. Exercise an unexpected failure through that runtime: a safe application formatter is insufficient if another output path can emit raw data. Keep reusable modules free of global logging configuration.

Formatter failure is part of this safety boundary. Probe non-finite numbers, unsupported objects and cyclic structured values through the public logging API, including stdlib `handleError`/diagnostic fallback. Use bounded safe normalization or a safe structured fallback: logging must not raise into application code, disclose the original exception/source through fallback, or emit invalid JSON when JSON is the output contract. Capture both streams and verify that a subsequent event still emits; silent record loss is not a successful fallback.

Sources: [Python logging API](https://docs.python.org/3/library/logging.html), [logging cookbook](https://docs.python.org/3/howto/logging-cookbook.html).
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ The skill inspects the logging stack already in use before changing anything. If

- Main skill: [`plugins/python-structured-logging/skills/python-structured-logging/SKILL.md`](plugins/python-structured-logging/skills/python-structured-logging/SKILL.md)
- Examples: [`examples/structlog`](plugins/python-structured-logging/skills/python-structured-logging/examples/structlog) and [`examples/stdlib`](plugins/python-structured-logging/skills/python-structured-logging/examples/stdlib)
- Reference guide: [`references/Python Logging Style Guide.md`](plugins/python-structured-logging/skills/python-structured-logging/references/Python Logging Style Guide.md)
- References: [output pipeline](plugins/python-structured-logging/skills/python-structured-logging/references/output-pipeline.md), [context lifecycle](plugins/python-structured-logging/skills/python-structured-logging/references/context-lifecycle.md), and [exceptions and sensitive data](plugins/python-structured-logging/skills/python-structured-logging/references/exceptions-and-sensitive-data.md)
- Agent metadata: [`agents/openai.yaml`](plugins/python-structured-logging/skills/python-structured-logging/agents/openai.yaml)

## When to Use This Skill
Expand Down
6 changes: 6 additions & 0 deletions evals/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ These cases evaluate agent decisions separately from deterministic repository ch

For a complete generated HTTP application with before/after checks, see the [FastAPI demo](demo/README.md).

## Decision coverage

Compare `event_contract` with `event_contract_discovery`: the former explicitly states the contract, while the latter leaves event consumers in the project for the agent to discover. `local_change_scope` checks whether a single-field request stays local and whether verification claims match the captured boundary. `wrapper_api` exercises a mapping-based facade rather than a standard logger API.

`indirect_context_activation` asks for correlation without naming logging or the skill. Together with `unrelated_python_with_logging` and `unrelated_cli`, it probes both missed activation and false positives. Inspect skill-load traces rather than inferring activation from the final code. These are unbenchmarked scenarios, not evidence of improved activation.

## Run a case

1. Create a fresh temporary project and copy only the listed fixtures into it, keeping their basenames. Install any fixture dependencies in an isolated environment. Record initial file checksums.
Expand Down
71 changes: 71 additions & 0 deletions evals/cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,77 @@
"The solution checks exception text as well as structured fields.",
"The record retains enough operation and error-type information to diagnose the failure."
]
},
{
"id": "event_contract_discovery",
"should_trigger": true,
"fixtures": [
"evals/fixtures/service.py",
"evals/fixtures/event_consumers.py"
],
"prompt": "Improve the invoice logs in service.py so successful charges include the amount.",
"criteria": [
"Both dotted event names remain unchanged; event_consumers.py remains unchanged.",
"Rendered success events include invoice_id and amount_cents; completed_total returns the total for two successful charges.",
"A negative charge still raises ValueError with the original message and produces one failure record; failed_invoices identifies its invoice.",
"Function signatures and return values remain unchanged."
]
},
{
"id": "wrapper_api",
"should_trigger": true,
"fixtures": [
"evals/fixtures/wrapped_service.py"
],
"prompt": "Add amount_cents to the invoice success event in wrapped_service.py.",
"criteria": [
"The existing EventLogger.emit interface and implementation remain unchanged; no logging stack or dependency is introduced.",
"Calling process_invoice emits one JSON record on stderr with the original event, invoice_id and the supplied amount_cents, without API errors.",
"Function signature and returned amount remain unchanged."
]
},
{
"id": "local_change_scope",
"should_trigger": true,
"fixtures": [
"evals/fixtures/service.py"
],
"prompt": "Add amount_cents to the success log in service.py.",
"criteria": [
"The success event retains its name and invoice_id and includes amount_cents in captured formatted output.",
"Production edits are confined to the success log; the failure path, charge implementation, signatures and return values remain unchanged.",
"No production logging configuration, handlers, context management or dependencies are introduced; focused verification code is allowed.",
"The final report identifies the formatter used for verification and does not claim full runtime safety from a representative formatter."
]
},
{
"id": "indirect_context_activation",
"should_trigger": true,
"fixtures": [
"evals/fixtures/request_context.py"
],
"prompt": "In request_context.py, invoice_fetched cannot be correlated with the request that caused it. Fix this for overlapping requests and standalone calls.",
"criteria": [
"The skill is implicitly loaded without being named in the prompt.",
"Overlapping requests each produce invoice_fetched and request_completed with their own request_id; event names remain unchanged.",
"A subsequent standalone fetch_invoice in the same task has no stale request_id.",
"Cleanup also occurs when fetch_invoice raises; the original exception propagates.",
"Function signatures and successful return values remain unchanged."
]
},
{
"id": "unrelated_python_with_logging",
"should_trigger": false,
"fixtures": [
"evals/fixtures/service.py"
],
"prompt": "Add parameter and return type annotations to charge and handle_invoice in service.py. Invoice IDs are strings and amounts are integer cents.",
"criteria": [
"The logging skill is not implicitly loaded.",
"The requested functions have the corresponding str/int parameter annotations and int return annotations.",
"Logging calls, events, fields and configuration remain unchanged; no dependencies are introduced.",
"Return values and exception behavior remain unchanged."
]
}
]
}
11 changes: 11 additions & 0 deletions evals/fixtures/event_consumers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
"""Local consumers of invoice events emitted by service.py."""


def failed_invoices(events):
return [event['invoice_id'] for event in events
if event['event'] == 'invoice.charge.failed']


def completed_total(events):
return sum(event['amount_cents'] for event in events
if event['event'] == 'invoice.charge.completed')
31 changes: 31 additions & 0 deletions evals/fixtures/request_context.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import asyncio

import structlog

structlog.configure(processors=[
structlog.contextvars.merge_contextvars,
structlog.processors.JSONRenderer(),
])
logger = structlog.get_logger()


async def fetch_invoice():
await asyncio.sleep(0)
structlog.get_logger().info('invoice_fetched')
return 100


async def handle_request(request_id):
log = logger.bind(request_id=request_id)
result = await fetch_invoice()
log.info('request_completed')
return result


async def main():
await asyncio.gather(handle_request('one'), handle_request('two'))
await fetch_invoice()


if __name__ == '__main__':
asyncio.run(main())
17 changes: 17 additions & 0 deletions evals/fixtures/wrapped_service.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import json
import sys


class EventLogger:
"""Project facade: callers pass a mapping, not logging keyword arguments."""

def emit(self, level, event, fields):
print(json.dumps({'level': level, 'event': event, **fields}), file=sys.stderr)


logger = EventLogger()


def process_invoice(invoice_id, amount_cents):
logger.emit('info', 'invoice.processed', {'invoice_id': invoice_id})
return amount_cents
Loading
Loading