chore: merge develop into main - #464
Merged
Merged
Conversation
Add three private helper functions (_sanitize_for_log, _safe_exception_repr, _safe_exception_str) to safely render exception messages and type names before logging them. These functions strip CR/LF, replace unpaired Unicode surrogates, truncate long text, and provide fallbacks for rendering failures. Co-Authored-By: Claude <noreply@anthropic.com>
Implements Task 3 of the ADDON-90004 plan: adds _DowngradeToInfoFilter class that downgrades WARNING/ERROR/CRITICAL records to INFO, plus tuples of logger names for OTel SDK internals that will be attached in Tasks 4-5. Co-Authored-By: Claude <noreply@anthropic.com>
Attach the _downgrade_to_info_filter to the 5 metrics-SDK loggers (opentelemetry.sdk.metrics.*) inside ObservabilityService.__init__. This suppresses WARNING/ERROR level log entries from the SDK and converts them to INFO, reducing observability noise. Co-Authored-By: Claude <noreply@anthropic.com>
…r noise
- Set GRPC_VERBOSITY=NONE at start of _create_otlp_exporter to suppress C-core
handshake diagnostics to stderr unless explicitly overridden by caller
- Attach _downgrade_to_info_filter to 4 _OTLP_LOGGERS before OTLPMetricExporter
construction to downgrade WARNING/ERROR logs from opentelemetry exporter and
utilities to INFO level, matching the filtering already applied to the 5
metrics SDK loggers during ObservabilityService.__init__
- Add comprehensive tests:
- test_create_otlp_exporter_sets_grpc_verbosity_when_absent: verifies NONE is set
- test_create_otlp_exporter_respects_existing_grpc_verbosity: verifies caller's
value is preserved
- test_create_otlp_exporter_attaches_filter_to_otlp_loggers: verifies filters
are present on the logger objects
- test_create_otlp_exporter_does_not_attach_filter_on_missing_port: verifies
filters only attached on the success path
- test_filters_are_attached_before_construction_logs_occur: regression guard
ensuring filter attachment happens before OTLPMetricExporter() runs, using
real grpc + malformed env vars to trigger actual WARNING logs
- TestGrpcTlsHandshakeStderrSuppression integration test: verifies that gRPC
C-core stderr is actually suppressed with GRPC_VERBOSITY=NONE
Add _CircuitBreakerExporter class that wraps an inner MetricExporter and stops calling it after 3 consecutive failed exports (via MetricExportResult.FAILURE or exception) within the same process. Once tripped, the exporter silently returns SUCCESS without calling the inner exporter or logging further messages. This eliminates WARNING/ERROR log noise from OTLP export failures while still recording that the 3rd failure occurred. Co-Authored-By: Claude <noreply@anthropic.com>
… access Add threading.Lock() to _CircuitBreakerExporter to prevent unsynchronized mutations of _tripped, _consecutive_failures, and _shutdown_called while concurrent threads may be accessing these fields. Wrap entire method bodies (export, force_flush, shutdown) to ensure atomicity of state checks and delegations to the inner exporter. Add deterministic concurrency test proving export() and shutdown() are now mutually exclusive: export blocks on a threading.Event, and shutdown() is demonstrated to block while the export holds the lock, then proceed normally after release. Co-Authored-By: Claude <noreply@anthropic.com>
Modified _create_otlp_exporter() to return the OTLP exporter wrapped in _CircuitBreakerExporter instead of returning it directly. This enables the circuit breaker to stop calling the inner exporter after 3 consecutive failed exports, eliminating WARNING/ERROR log noise from solnlib.observability when the Spotlight collector becomes unavailable. Updated test_create_otlp_exporter_returns_exporter_when_cert_present to verify the wrapper is returned. Added test_create_otlp_exporter_wrapper_forwards_shutdown to verify the wrapper properly delegates shutdown calls to the inner exporter. Co-Authored-By: Claude <noreply@anthropic.com>
Add input validation for event_count and byte_count arguments to StanzaObservabilityRecorder.record() to reject invalid values (negative numbers, non-ints, out-of-int64-range values) and log them at INFO level instead of raising or silently forwarding to the OTel SDK. Co-Authored-By: Claude <noreply@anthropic.com>
Add _attr_key_error and _attr_value_error helper functions to validate OpenTelemetry attribute keys and values per OTEL spec (string keys, bool/int/float/string values). Modify StanzaObservabilityRecorder.record to validate and filter extra_attrs, dropping invalid entries and preventing override of splunk.modinput.name. Co-Authored-By: Claude <noreply@anthropic.com>
Add validation in StanzaObservabilityRecorder.__init__ to reject modinput_type and stanza_name that are not strings or contain CR/LF. Raise TypeError before cache lookup to prevent invalid entries. Replace all 5 self._service._logger.info(...) calls in the record() method with self._logger.info(...) so the recorder logs on its own behalf rather than reaching into the service's logger. Co-Authored-By: Claude <noreply@anthropic.com>
…ce callers Co-Authored-By: Claude <noreply@anthropic.com>
Code-quality review of the modinput_type graceful-degrade test flagged that it only checked logger.info was called, not that warning/error were absent -- the actual contract this ticket is about. Co-Authored-By: Claude <noreply@anthropic.com>
…own calls _CircuitBreakerExporter.export()/force_flush()/shutdown() held the state lock across the blocking calls into the inner OTLP exporter, so shutdown() could be blocked for the full duration of an in-flight export instead of promptly reaching the inner exporter's own shutdown-triggered retry interruption.
## Summary - Cap all of `solnlib.observability`'s own log calls at INFO and downgrade known OpenTelemetry OTLP/metrics-SDK logger output to INFO as well, so export failures no longer spam ERROR/WARNING into add-on logs — this preserves the diagnostic message rather than discarding it. - Suppress gRPC C-core stderr TLS noise via `GRPC_VERBOSITY`, and add safe exception rendering helpers (`_safe_exception_str`/`_safe_exception_repr`/`_sanitize_for_log`) so caught errors can never raise, leak unpaired surrogates, or inject extra log lines. - Wrap the OTLP metric exporter in a circuit breaker that stops calling the inner exporter after 3 consecutive failures in a process, catches ordinary exceptions from the inner exporter so they never escape to `PeriodicExportingMetricReader`, and preserves temporality/aggregation preferences through the wrapper. - Validate recorder inputs before they reach OTel: `event_count`/`byte_count`, `extra_attrs` keys/values, and recorder identity strings (raises `TypeError` for `StanzaObservabilityRecorder`, gracefully degrades for direct `ObservabilityService(modinput_type=...)` callers instead of crashing or silently corrupting metrics). - Fix a concurrency bug in `_CircuitBreakerExporter`: the state lock was held across the blocking `export()`/`force_flush()`/`shutdown()` calls into the inner exporter, so `shutdown()` could be blocked for the full duration of an in-flight export instead of promptly reaching the inner exporter's own retry-interrupt signaling. The lock now only guards breaker-state reads/writes. ## Ticket [ADDON-90004](https://splunk.atlassian.net/browse/ADDON-90004) ## Known, intentional trade-off An external review flagged that `_DowngradeToInfoFilter` doesn't re-check the logger's effective level after downgrading a record to INFO, so a downgraded record can still be written even when the add-on has explicitly raised its log level above INFO to suppress noise. This was investigated and left as-is: fixing it would drop these OTel diagnostics by default for *every* add-on that never explicitly configures logging (root defaults to WARNING) — Python's logging module can't distinguish "WARNING by default" from "WARNING by deliberate `Logs().set_level()` call," so honoring the latter necessarily breaks the former. That default-visibility behavior is an explicit goal of the design and is covered by `test_filters_are_attached_before_construction_logs_occur`. Revisit only as a deliberate design change, not a bugfix. ## Test plan - [x] `pytest tests/unit -v` — 339 passed - [x] `pre-commit run --files solnlib/observability.py tests/unit/test_observability.py` — clean - [x] Holistic code review pass — ready to merge
# [8.1.0-beta.2](v8.1.0-beta.1...v8.1.0-beta.2) (2026-08-20) ### Bug Fixes * ADDON-90004 add circuit breaker for the OTLP exporter ([cfb5696](cfb5696)) * ADDON-90004 add downgrade-to-INFO logging filter ([2c18948](2c18948)) * ADDON-90004 add safe log-rendering helpers to observability module ([04473b4](04473b4)) * ADDON-90004 cap solnlib.observability log calls at INFO ([4d40269](4d40269)) * ADDON-90004 change observability log level ([#461](#461)) ([5a1fa77](5a1fa77)) * ADDON-90004 don't hold breaker lock across blocking export/shutdown calls ([572db48](572db48)) * ADDON-90004 downgrade metrics SDK logger noise to INFO ([c8bbc5d](c8bbc5d)) * ADDON-90004 suppress gRPC C-core stderr and downgrade OTLP logger noise ([fb967ab](fb967ab)) * ADDON-90004 synchronize circuit breaker state against concurrent access ([5c11333](5c11333)) * ADDON-90004 validate event_count/byte_count before recording ([80606b8](80606b8)) * ADDON-90004 validate extra_attrs keys and values before recording ([3b54dc1](3b54dc1)) * ADDON-90004 validate modinput_type for direct ObservabilityService callers ([8761cf7](8761cf7)) * ADDON-90004 validate recorder identity strings, raise TypeError ([0eef64d](0eef64d)) * ADDON-90004 wrap the OTLP exporter in the circuit breaker ([bdc76b3](bdc76b3))
Merges main into develop to resolve version conflicts blocking PR #462. Conflicts were in pyproject.toml and solnlib/__init__.py (version strings: 8.1.0 on main vs 8.1.0-beta.2 on develop). Kept develop's version (8.1.0-beta.2).
## Summary - Merges `main` into `develop` to resolve version conflicts blocking PR #462 - Conflicts were in `pyproject.toml` and `solnlib/__init__.py` (version strings: `8.1.0` on main vs `8.1.0-beta.2` on develop) - Kept develop's version (`8.1.0-beta.2`) ## Context PR #462 (`main` → `develop`) is blocked with a `CONFLICTING` merge state because `main` has the `chore(release): 8.1.0` commit that isn't in `develop`. `main` is protected against direct pushes, so the conflict is resolved here instead, on a branch off `develop`. Once this PR merges, PR #462 will be conflict-free (mirrors the pattern used in #455 and #459). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
## [8.1.1-beta.1](v8.1.0...v8.1.1-beta.1) (2026-08-20) ### Bug Fixes * ADDON-90004 add circuit breaker for the OTLP exporter ([cfb5696](cfb5696)) * ADDON-90004 add downgrade-to-INFO logging filter ([2c18948](2c18948)) * ADDON-90004 add safe log-rendering helpers to observability module ([04473b4](04473b4)) * ADDON-90004 cap solnlib.observability log calls at INFO ([4d40269](4d40269)) * ADDON-90004 change observability log level ([#461](#461)) ([5a1fa77](5a1fa77)) * ADDON-90004 don't hold breaker lock across blocking export/shutdown calls ([572db48](572db48)) * ADDON-90004 downgrade metrics SDK logger noise to INFO ([c8bbc5d](c8bbc5d)) * ADDON-90004 suppress gRPC C-core stderr and downgrade OTLP logger noise ([fb967ab](fb967ab)) * ADDON-90004 synchronize circuit breaker state against concurrent access ([5c11333](5c11333)) * ADDON-90004 validate event_count/byte_count before recording ([80606b8](80606b8)) * ADDON-90004 validate extra_attrs keys and values before recording ([3b54dc1](3b54dc1)) * ADDON-90004 validate modinput_type for direct ObservabilityService callers ([8761cf7](8761cf7)) * ADDON-90004 validate recorder identity strings, raise TypeError ([0eef64d](0eef64d)) * ADDON-90004 wrap the OTLP exporter in the circuit breaker ([bdc76b3](bdc76b3))
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04e335ac31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [tool.poetry] | ||
| name = "solnlib" | ||
| version = "8.1.0" | ||
| version = "8.1.0-beta.2" |
There was a problem hiding this comment.
When this commit is applied to its d574790 parent, it changes the package version from the already released 8.1.0 back to 8.1.0-beta.2. Since that prerelease sorts before 8.1.0, builds from this mainline commit and Git-based installations advertise an older version and may not satisfy dependencies requiring solnlib>=8.1.0; keep the stable version until release automation assigns a newer one. The matching solnlib.__version__ change has the same problem.
Useful? React with 👍 / 👎.
| - Any other exception occurs during exporter construction (including a | ||
| missing ``grpcio`` package). | ||
| """ | ||
| os.environ.setdefault("GRPC_VERBOSITY", "NONE") |
There was a problem hiding this comment.
When Spotlight has no discoverable port or certificate, this line still permanently sets the process-wide GRPC_VERBOSITY setting even though no OTLP exporter is constructed. An add-on that later initializes an unrelated gRPC client or server then loses all gRPC C-core diagnostics merely because it attempted to enable observability; resolve the port and certificate first, and only apply this global setting immediately before importing/initializing gRPC for an exporter.
Useful? React with 👍 / 👎.
rmrowczynski-splunk
approved these changes
Aug 20, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
No description provided.