fix(docs): correct flush()/reset() lifecycle documentation (SAO-16166) - #232
Open
etserend wants to merge 8 commits into
Open
fix(docs): correct flush()/reset() lifecycle documentation (SAO-16166)#232etserend wants to merge 8 commits into
etserend wants to merge 8 commits into
Conversation
…o examples (SAO-16166) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The drain-only rewording in the previous commit left three claims that do not match the code. Teardown: SAO-16166 asked each docstring to name terminate() as the teardown call, but terminate() is defined only on SplunkAOLogger. Neither SplunkAOLoggerSingleton nor SplunkAODecorator exposes it, so that advice would raise AttributeError if followed. Point at each layer's real teardown instead -- reset()/reset_all() on the singleton, reset() on the decorator -- and note that atexit already terminates loggers at interpreter exit, so an explicit call is for deterministic shutdown rather than an obligation. SplunkAODecorator.flush_all() names none, since the decorator has no reset_all() and reset() only covers the current context. Hook path: flush() is not drain-only when an ingestion hook is configured. It concludes open spans, computes local metrics, delivers the accumulated traces to the hook, and clears them. Document flush() and async_flush() as two egress paths and carry the caveat into the wrappers that can reach a hook-backed logger. SplunkAODecorator.flush() is excluded on purpose: it resolves a logger without forwarding an ingestion hook, so drain-only holds there unconditionally. Scoping: SplunkAOLoggerSingleton.flush() never drained every cached logger. The cache key includes the thread name and mode, and routing falls back to deployment defaults, so a bare call is far narrower than documented. Also drop a stale "terminate and clear" comment in flush_all() that contradicted the docstring above it. The three-line reformat in logger.py's constructor is pre-existing and was applied by the ruff-format hook. Co-Authored-By: Claude <noreply@anthropic.com>
These files predate the current ruff config and trip the pre-commit hooks as soon as they are touched. Landing the automated fixes on their own keeps the churn out of the docs change that follows. All hook-applied: isort ordering, the redundant "r" mode in open(), and one collapsed call. No behavior or wording changes. Co-Authored-By: Claude <noreply@anthropic.com>
…on (SAO-16166) The chatbot samples told readers that flushing is what starts the next trace. It is not: flush() leaves trace state alone on the OTLP path, and only clears the current parent when an ingestion hook is configured. What actually frees the next start_trace() at the top of the loop is conclude(). Reword the comment to attribute each call correctly and to say what flush() is really for here -- exporting immediately rather than waiting out the batch processor's schedule delay. Comments only; the surrounding conclude()/flush() calls are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
The trailing-whitespace hook rewrites this file as soon as it is touched. Landing it alone keeps the churn out of the docs change that follows. Note these two were Markdown hard line breaks, so the affected blockquote and paragraph lines now flow together rather than breaking. They were the only two such breaks in the file, so this normalizes them. Co-Authored-By: Claude <noreply@anthropic.com>
Restoring logger.flush() to the galileo "Before" snippet made the delta visible, but the guide never said what changed: flush() appeared nowhere outside the two code blocks in section 7, and section 5 "Removed / Changed Features" did not cover it. A reader's only signal was a line going missing between two 25-line examples. Add 5.4 stating that conclude() enqueues and the atexit hook exports at interpreter exit, so no explicit call is needed -- while being clear that flush() still exists and still exports, for callers who want spans out ahead of the batch timer, and that terminate() is the deterministic shutdown. Also point the "After" block at 5.4, and fix a stale log_stream reference in its comment that should have read agent_stream. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
fercor-cisco
changed the title
Found while correcting the flush() docstrings: the same wrong mental model had produced two more inaccurate claims about reset(), and the class docstring that seeded them. SplunkAODecorator.reset() claimed it "deletes all traces that haven't been flushed". It does not. reset() terminates the context's loggers, and terminate() calls force_flush() before shutdown(), so anything already concluded is exported. Only spans still open are discarded. SplunkAOLoggerSingleton.reset() claimed it resets "one or all" instances. Like flush(), it prefix-matches a full key, so with no arguments it covers the current thread at the default mode and resolved routing. reset_all() is the "all" API. The prefix match does pull in the per-trace and hook-backed variants, which is worth stating. The class docstring described the cache key as a (project, agent_stream) tuple. It is really (thread name, mode, deployment, project, agent stream or experiment) plus trace and span IDs when present -- which is why "all cached loggers" kept getting written. Documenting the thread component also explains why instances are never shared across threads. Also drop an overstatement in three chatbot examples: flush() was described as ensuring delivery, but it runs with a timeout, degrades failures to a logged warning, and is a no-op under SPLUNK_AO_LOGGING_DISABLED. It exports immediately; it does not guarantee. Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
|
Follow-up commits, docs only.
Migration guide — new §5.4: The two |
Only splunk-ao-migration-tool/README.md conflicted, where #234 rewrote 5.3 at the point this branch inserted 5.4. Kept both: main's SPLUNK_AO_HOME_DIR sentence closes 5.3, then 5.4 follows. Also dropped this branch's log_stream -> agent_stream fix in the "After" block, since #234 made the identical change. Co-Authored-By: Claude <noreply@anthropic.com>
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
2 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.
Summary
Follow-up to #217 (HYBIM-966). PR #217 removed stale
flush()calls from examples but did not update the four public docstrings its description promised, and incorrectly removedlogger.flush()from the galileo "Before" migration snippets.singleton.py/decorator.py— four docstrings: replace "upload and clear" / "Upload all captured traces" with drain-only wordingsplunk-ao-migration-tool/examples/before_galileo.py— restorelogger.flush() # uploads traces(galileo's flush genuinely uploads; the line correctly shows what callers remove when migrating)splunk-ao-migration-tool/README.md— same restoration in the Before (galileo) block; After (splunk-ao) block is unchangedNo behavior changes. Docs and docstrings only.
Closes https://splunk.atlassian.net/browse/SAO-16166
🤖 Generated with Claude Code