Add scenario overrides for topology-driven logs#156
Merged
Conversation
Scenarios can now modify log output during their activation window via a logs: block under override, mirroring the metric override shape. add: defines extra log records (full LogConfig: severity, body templates, conditions, probability, timing anchors, attributes) emitted only while the scenario is active; disable: true mutes the base logs (topology templates and derived error/slow logs) at the override's scope. Overrides work at both scopes: service.operation refs and bare service names, consistent with metric overrides. Added definitions are validated at config load with contextual error messages, resolved in BuildScenarios, merged across overlapping scenarios (adds accumulate, disable from any active scenario wins), and applied by LogObserver via the existing OverrideObserver notification path. Closes #155
There was a problem hiding this comment.
Pull request overview
Adds scenario-time-window overrides for topology-driven logs, mirroring the existing scenario override model for metrics, so incident scenarios can add extra log templates and/or mute base logs for a given service or operation scope.
Changes:
- Generalize
resolveLogserror-context handling to support scenario override contexts. - Introduce
logs:scenario override support (add,disable) across config validation, scenario building/merging, and runtime observer application. - Extend unit tests and docs example(s) to cover scenario log override behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/synth/topology.go | Generalizes resolveLogs API to accept an explicit error-context string. |
| pkg/synth/scenario.go | Carries parsed/merged log overrides (AddLogs, DisableLogs) through scenario resolution. |
| pkg/synth/scenario_test.go | Adds unit tests for parsing and merging scenario log overrides. |
| pkg/synth/logs.go | Implements scenario-aware log emission (mute base logs + emit added templates) via SetOverrides. |
| pkg/synth/logs_test.go | Adds LogObserver tests for add/disable semantics and derived-log interactions. |
| pkg/synth/config.go | Adds LogOverrideConfig and validates scenario logs: overrides at config load time. |
| pkg/synth/config_test.go | Adds validation tests for scenario logs: overrides at both scopes. |
| docs/examples/topology-driven-logs.yaml | Demonstrates logs.add and logs.disable scenario override usage. |
| docs/examples/README.md | Documents the updated topology-driven logs example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review: SetOverrides silently skipped newLogTemplate errors, which could mask a future drift between validLogSeverity and severityByName. Derive validLogSeverity from severityByName so the validation set cannot diverge from the severities the observer can emit, and make newLogTemplate infallible: severity is rejected at config load, and a hand-constructed unknown severity maps to SeverityUndefined while preserving the severity text rather than dropping the record. Also add coverage for operation-scoped disable muting derived logs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #155. Follow-up to PR 145 (topology-driven log generation), mirroring the metric override pattern from
OverrideConfig.Metrics.What
Scenarios can now modify log output during their activation window via a
logs:block underoverride:add:defines extra log records emitted only while the scenario window is active. Entries are fullLogConfigdefinitions: severity, body templates, conditions (error/success/slow), probability, timing anchors (at/delay), and attribute generators all work.disable: truemutes the base logs at the override's scope for the duration of the window — both topology log templates and the derived error/slow fallback logs. Combined withadd:, this gives replacement semantics.service.operationrefs scope to one operation; bare service names scope to every span of the service.Design decisions (per the issue's open questions)
add:anddisable: trueonly; noname/idfield was added toLogConfig. Log-storm simulation is achievable today withdisable: trueplus anadd:entry that copies the base log with a higher probability.disablemutes the derived logs too — so the suppression rule is now scenario-aware.Implementation
LogOverrideConfig(add,disable) onOverrideConfig, validated at config load with the usual contextual messages (eachaddentry goes throughvalidateLogConfig; an emptylogs:block is rejected).BuildScenariosresolves added entries toLogDefinitions via the existingresolveLogs(its error-context parameter was generalised to support scenario contexts).ResolveOverridesmerges across overlapping scenarios: adds accumulate in priority order, disable from any active scenario wins.LogObserverimplements the existingOverrideObserverinterface; the engine'snotifyOverridespath needed no changes. Added definitions are pre-built into templates on scenario transitions so the per-span path only does map lookups under an RWMutex, matchingMetricObserver.Testing
docs/examples/topology-driven-logs.yaml: the added ERROR log appears only inside the window on error spans, the gateway's base logs go quiet during it, and behaviour reverts when the window closes.make testandmake lintpass.https://claude.ai/code/session_01L12XgTDZiZMBQU6cMoFLpC
Generated by Claude Code