Conversation
Complete the mechanical terminology rename for SplunkAOLogger params, Agent Control targets, OTLP routing headers, and resource attributes. Co-authored-by: Cursor <cursoragent@cursor.com>
Fix missed SplunkAOLogger call sites, TracesIngestRequest assertions, user-facing terminology, legacy routing attr stripping, and README/examples. Co-authored-by: Cursor <cursoragent@cursor.com>
…format Expand agent stream terminology across SDK, examples, and tests outside resources/, but keep OTLP headers, resource attributes, and Agent Control target_type aligned with the current backend API. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep agent_stream SDK rename while adopting main's Splunk AO branding. Fix splunk-ao-logger redaction example and exporter config test after merge. Co-authored-by: Cursor <cursoragent@cursor.com>
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-4-8). It may contain mistakes.
Verdict: request_changes — Rename leaked into a generated-client call, passing a nonexistent kwarg that will crash AgentStream.refresh()/get(id=...) at runtime.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
src/splunk_ao/agent_streams.py:409-415: Add a unit test that exercises AgentStreams.get(id=...) by patching the generatedget_log_stream_projects_project_id_log_streams_log_stream_id_get.syncfunction (rather than mocking the whole AgentStreams class). The current refresh() tests mock AgentStreams entirely, so no test ever calls the real generated client — which is exactly why this TypeError shipped undetected.
| if id: | ||
| log_stream_response = get_log_stream_projects_project_id_log_streams_log_stream_id_get.sync( | ||
| project_id=project_id, log_stream_id=id, client=self.config.api_client | ||
| project_id=project_id, agent_stream_id=id, client=self.config.api_client |
There was a problem hiding this comment.
🔴 critical (bug): The rename leaked into the OpenAPI-generated client call — which the PR description explicitly states must stay untouched. The generated function signature is sync(project_id: str, log_stream_id: str, *, client) (see src/splunk_ao/resources/api/log_stream/get_log_stream_projects_project_id_log_streams_log_stream_id_get.py). It has no agent_stream_id parameter and no **kwargs, so this call raises TypeError: sync() got an unexpected keyword argument 'agent_stream_id' (and a missing-required-argument error for log_stream_id).
This breaks every AgentStreams.get(id=...) lookup at runtime — most notably AgentStream.refresh() (agent_stream.py:386). The existing refresh tests don't catch it because they mock AgentStreams wholesale, so this call path is never exercised. Revert the keyword to log_stream_id=id (only the local variable id was renamed elsewhere; the generated parameter name was not).
| project_id=project_id, agent_stream_id=id, client=self.config.api_client | |
| log_stream_response = get_log_stream_projects_project_id_log_streams_log_stream_id_get.sync( | |
| project_id=project_id, log_stream_id=id, client=self.config.api_client | |
| ) |
🤖 Generated by the Astra agent
The OpenAPI client expects log_stream_id, not agent_stream_id; passing the renamed kwarg broke AgentStreams.get(id=...) and AgentStream.refresh(). Add a regression test that exercises the generated sync call directly. Co-authored-by: Cursor <cursoragent@cursor.com>
…nt call" This reverts commit ad5db52.
The OpenAPI client expects log_stream_id, not agent_stream_id; the renamed kwarg broke AgentStreams.get(id=...) and AgentStream.refresh(). Co-authored-by: Cursor <cursoragent@cursor.com>
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-4-8). It may contain mistakes.
Verdict: approve — Rename is thorough, internally consistent, and preserves backend wire format; the one critical bug was already reverted. Remaining issues are example/doc-level only.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
src/splunk_ao/agent_streams.py:409-415: The existingrefresh/get(id=...)tests mockAgentStreamswholesale, so the generated-client call path (get_log_stream_projects_project_id_log_streams_log_stream_id_get.sync) is never exercised — which is why the earlier keyword regression slipped through. Consider adding a test that mocks the generatedsyncfunction directly and asserts it is called withlog_stream_id=, to guard the SDK↔generated-client boundary against future renames.
| with galileo_context(project="my-project", agent_stream="production"): | ||
| result = call_llm("Hello") | ||
|
|
||
| # Direct logger approach | ||
| # project/log_stream are constructor args, not start_session args | ||
| logger = GalileoLogger(project="my-project", log_stream="production") | ||
| logger = GalileoLogger(project="my-project", agent_stream="production") |
There was a problem hiding this comment.
🟡 minor (bug): This is the "before" example demonstrating the real Galileo SDK (from galileo import GalileoLogger, galileo_context). Galileo's API uses the log_stream= keyword — it has no agent_stream= parameter. The rename over-reached into this file, so both galileo_context(..., agent_stream="production") and GalileoLogger(project=..., agent_stream="production") would raise TypeError: unexpected keyword argument 'agent_stream' if a user ran them. The pre-migration snippet should keep Galileo's real terminology. (The same over-reach appears in the migration-tool README's "before Galileo" code block.)
| with galileo_context(project="my-project", agent_stream="production"): | |
| result = call_llm("Hello") | |
| # Direct logger approach | |
| # project/log_stream are constructor args, not start_session args | |
| logger = GalileoLogger(project="my-project", log_stream="production") | |
| logger = GalileoLogger(project="my-project", agent_stream="production") | |
| with galileo_context(project="my-project", log_stream="production"): | |
| result = call_llm("Hello") | |
| # Direct logger approach | |
| # project/log_stream are constructor args, not start_session args | |
| logger = GalileoLogger(project="my-project", log_stream="production") |
🤖 Generated by the Astra agent
There was a problem hiding this comment.
README.md:82-82 (line not in diff)
🟡 minor (documentation): This PR makes SPLUNK_AO_AGENT_STREAM the preferred env var and demotes SPLUNK_AO_LOG_STREAM to a deprecated alias, but the primary setup instructions here (and the accompanying comments on lines 103/134) still point new users at the deprecated name. Recommend updating the shell example and comments to SPLUNK_AO_AGENT_STREAM so docs steer users to the non-deprecated variable. The same applies to splunk-ao-migration-tool/examples/after_splunk_ao.py, which sets os.environ["SPLUNK_AO_LOG_STREAM"].
| export SPLUNK_AO_AGENT_STREAM="your-agent-stream-name" |
🤖 Generated by the Astra agent
…REAM Restore log_stream= in before-Galileo examples (real Galileo API) and document SPLUNK_AO_AGENT_STREAM as the preferred env var, with SPLUNK_AO_LOG_STREAM noted as a deprecated alias. Co-authored-by: Cursor <cursoragent@cursor.com>
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-4-8). It may contain mistakes.
Verdict: approve — Mechanical rename is consistent and correct; backend wire formats preserved as intended; the one critical bug was already fixed. Only stale-docstring nits remain.
General Comments
- 🟡 minor (documentation): Several docstrings still document parameters by their old names (
log_stream,log_stream_id) even though the actual keyword parameters were renamed toagent_stream/agent_stream_id. Examples:src/splunk_ao/utils/singleton.pyreset()(lines ~255-258) andflush()(lines ~310-313) Parameters sections;src/splunk_ao/utils/singleton.py_get_keydocstring ("The log stream name"/"The log stream ID");src/splunk_ao/decorator.pyget_logger_instance/flush/initdocstrings referencinglog_stream. Since HYBIM-935's acceptance criteria is that nolog_streamspelling remains in SDK source (outside the intentional wire-format exceptions), these stale docstrings that name non-existent parameters should be updated to avoid confusing users. Non-blocking.
Follow-ups
Suggested follow-up work that could be tracked as Jira tickets:
tests/test_config.py:40-43: Consider adding an explicit test that when both SPLUNK_AO_AGENT_STREAM and the deprecated SPLUNK_AO_LOG_STREAM are set, the non-deprecated var wins (both in the env-var bridge and in_get_agent_stream_from_env/_get_agent_stream_or_default). The precedence relies on_BRIDGEordering andgetenv(new) or getenv(old)fallback logic that is newly introduced by this PR but not directly asserted.
Summary
Renames log stream → agent stream across the Splunk AO Python SDK public API, internal helpers, examples, ADK, and tests — while keeping all backend wire formats unchanged until
rungalileo/apiadopts the new terminology.This is a SDK-only breaking change. OTLP headers, OTLP resource attributes, REST JSON field names, and Agent Control
target_typevalues remain aligned with the current backend.Motivation
HYBIM-935 aligns SDK terminology with the product rename without breaking live integrations. Backend OpenAPI still uses
log_stream_id; OTLP routing still expectslogstream/logstreamidheaders andsplunk_ao.logstream.*resource attributes.Breaking changes
Context & logger
Traces, export, search, evaluators, annotation queues
Agent Control & OTel
Constants
Environment variables
Intentionally unchanged (backend wire format)
logstream,logstreamidsplunk_ao.logstream.name,splunk_ao.logstream.idlog_stream_idtarget_type"log_stream"src/splunk_ao/resources/**untouched/log_streams/...unchangedScope
85 files changed (+805 / −804)
logger,decorator,singleton,traces,export,search,evaluators,annotation_queues,agent_control,otel,exportersplunk-ao-adkplugin/callback/observerCommits
ea9e180— Initial SDK API renamee2b36b7— Review fixes (missed test params, README/examples)8cea2ee— Extend rename outsideresources/; preserve backend wire format