HYBIM-822 Changing headers in examples#58
Conversation
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 — Faithful mechanical rename; example .env files match code that already reads SPLUNK_AO_*, and the two SDK header renames are localized and consistent. Main dependency (upstream accepting the new header) is coordinated and gated by the migration doc.
General Comments
- 🟠 major (question): This PR flips the live ingest auth header from
Galileo-API-KeytoSplunk-AO-API-Keyinsrc/splunk_ao/traces.py(IngestTraces) andsrc/splunk_ao/otel.py(OTLP exporter). These are the actual authentication headers sent on every trace/span ingest request, so if the upstream ingest service does not yet acceptSplunk-AO-API-Keyin the environment this SDK targets, all ingestion will fail with 401. The description references api/#6788 and orbit/#985 as adding support — can you confirm those are deployed (not just merged) to the environments these examples point at (api.galileo.ai) before this merges? The design doc (docs/SPLUNK_AO_ENV_RENAME.md) also carries a "Do not merge until migration complete" notice worth honoring here.
Minor consistency note: the code uses Splunk-AO-API-Key while the PR description and upstream PRs spell it Splunk-AO-Api-Key. HTTP header names are case-insensitive (RFC 7230) so this won't break, but worth confirming the upstream allow-list isn't doing a case-sensitive string compare.
- 🟡 minor (testing): The
Splunk-AO-API-Keyheader rename inIngestTraces._headers(traces.py:188) and the OTLP exporter (otel.py:143) has no test asserting the new header name. The existingtests/test_traces_client_headers.pyonly covers theSplunk-AO-SDKheader on theTracesclass, not the*-API-Keyheader onIngestTraces/SplunkAOOTLPExporter. Since this is the auth-critical change in the PR, a small assertion that the constructed headers dict containsSplunk-AO-API-Key(and no longerGalileo-API-Key) would guard against a future revert/typo.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
README.md:29-34: Top-level README still documentsGALILEO_API_KEY,GALILEO_PROJECT,GALILEO_LOG_STREAM,GALILEO_LOGGING_DISABLED,GALILEO_CONSOLE_URL(lines 29-34, 46, 77, 180). This PR scope is examples + SDK headers, but the user-facing README will be inconsistent with the renamed examples once this lands. Worth a follow-up to rename these (or confirm it's covered by the deferred config.py/resources PR).openapi.yaml:41595-41595: The bundled OpenAPI spec still declares the APIKeyHeader security scheme asname: Galileo-API-Key. If the spec is meant to track the upstream API, it should be regenerated toSplunk-AO-API-Keyalongside the header rename. Likely belongs with the upstream spec regeneration rather than this PR.
Summary
README, log-mcp-calls, otel-collector config) and in the SDK source (src/splunk_ao/traces.py, src/splunk_ao/otel.py)
Details
The following header/env renames are covered:
│ Old │ New │
│ GALILEO_API_KEY │ SPLUNK_AO_API_KEY │
│ GALILEO_CONSOLE_URL │ SPLUNK_AO_CONSOLE_URL │
│ GALILEO_PROJECT │ SPLUNK_AO_PROJECT │
│ GALILEO_LOG_STREAM │ SPLUNK_AO_LOG_STREAM │
│ Galileo-API-Key (HTTP header) │ Splunk-AO-API-Key │
The upstream API and ingest-service now accept Splunk-AO-Api-Key (api/pull/6788, orbit/pull/985).
Notes
a separate PR