-
Notifications
You must be signed in to change notification settings - Fork 3
fix(config): HYBIM-960 derive standalone API URLs from custom console hosts #212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -60,14 +60,19 @@ export SPLUNK_AO_API_KEY="your-agent-observability-api-key" | |||||||||||||||||||||||
| export SPLUNK_AO_CONSOLE_URL="https://console.subdomain.yourcompany.com" | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| For standalone custom domains, the SDK derives the API hostname by replacing a | ||||||||||||||||||||||||
| leading `console.` or `app.` label with `api.`, or by adding an `api.` prefix | ||||||||||||||||||||||||
| when neither label is present. Set `SPLUNK_AO_API_URL` explicitly when your API | ||||||||||||||||||||||||
| does not follow this convention. | ||||||||||||||||||||||||
|
Comment on lines
+63
to
+66
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 minor (documentation): This paragraph documents the Worth one more sentence here, especially since
Suggested change
🤖 Generated by the Astra agent |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| > [!TIP] | ||||||||||||||||||||||||
| > Logging your first trace to on-premises Agent Observability? [Visit this guide](https://agent-observability-docs.splunk.com/sdk-redirect/on-prem-first-trace). | ||||||||||||||||||||||||
| > Logging your first trace to on-premises Agent Observability? [Visit this guide](https://agent-observability-docs.splunk.com/sdk-redirect/on-prem-first-trace). | ||||||||||||||||||||||||
| > | ||||||||||||||||||||||||
| > Learn how to find each environment variable in [this guide](https://agent-observability-docs.splunk.com/sdk-redirect/on-prem-keys). | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #### Splunk Observability (O11y) Cloud | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| > [!NOTE] | ||||||||||||||||||||||||
| > [!NOTE] | ||||||||||||||||||||||||
| > As of August 2026, Agent Observability on Splunk Observability Cloud is not yet generally available. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| | Environment variable | Description | | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -46,6 +46,25 @@ def resolve_deployment() -> DeploymentMode: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def resolve_standalone_api_url(console_url: str, api_url: str | None = None) -> str: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Return the explicit or console-derived standalone API base URL.""" | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if api_url: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return api_url | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if "localhost" in console_url or "127.0.0.1" in console_url: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "http://localhost:8088" | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+54
to
+55
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 minor (bug): The PR description says "Kept localhost development behavior and the
Routing to the API port rather than the console port is very likely the intended outcome, so I'm not asking you to revert it — but it should be stated in the description and the CHANGELOG rather than described as unchanged, and it deserves a test (see the Separately, 🤖 Generated by the Astra agent
Comment on lines
+54
to
+55
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 minor (bug): This over-broad match is inherited verbatim from galileo-core's hostname = urlsplit(base_url if "://" in console_url else f"https://{console_url}").hostname or ""
if hostname in ("localhost", "127.0.0.1", "::1") or hostname.endswith(".localhost"):
return "http://localhost:8088"At minimum this deserves a parametrized negative case ( 🤖 Generated by the Astra agent |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base_url = console_url.rstrip("/") | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if "://" not in base_url: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base_url = f"https://{base_url}" | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base_url = base_url.replace("://console.", "://api.", 1).replace("://app.", "://api.", 1) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scheme, host = base_url.split("://", 1) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not host.startswith("api."): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| host = f"api.{host}" | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+61
to
+63
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 minor (bug): The function's contract is to return an API base URL, so the console's path component should not survive. Pre-PR this was harmless (the host was unchanged, so the path was at least still valid for that origin); now both the host and the path are wrong. Deriving from the parsed hostname — as suggested in the comment on lines 57-65 — drops the path as a side effect. If you'd rather keep the string manipulation, strip the path explicitly: host = host.partition("/")[0]A parametrized case such as 🤖 Generated by the Astra agent |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return f"{scheme}://{host}" | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+57
to
+65
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 major (bug): This is reachable, and it is a regression rather than a pre-existing wart:
A scheme-less console URL is a realistic user input: several example
Suggested change
🤖 Generated by the Astra agent
Comment on lines
+57
to
+65
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 major (bug): The Concrete failures:
IP-literal and single-label (k8s service / hostname) console URLs are the normal shape for on-prem single-ingress installs — the repo's own fixtures and docs use exactly this layout (
Note this also fixes the sibling case where the host part carries a port or path into the
Suggested change
🤖 Generated by the Astra agent |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @dataclass | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class O11yConfig: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Configuration for a Splunk Observability Cloud deployment.""" | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -150,5 +169,5 @@ def from_env(cls) -> "StandaloneConfig": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @property | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def otlp_endpoint(self) -> str: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Return the explicit or console-derived OTLP trace endpoint.""" | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base = self.api_url or self.console_url.replace("://console.", "://api.", 1).replace("://app.", "://api.", 1) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base = resolve_standalone_api_url(self.console_url, self.api_url) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return f"{base.rstrip('/')}/otel/v1/traces" | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 minor (testing): The new
Cases 1 and 3 are both inside the problem this PR set out to solve, so they belong in this PR rather than a follow-up. 🤖 Generated by the Astra agent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 major (documentation): This is filed under
### Fixed, but for a class of existing users it is a breaking change, not a fix. Any standalone deployment whose console host lacks aconsole./app.prefix previously hadapi_urlfall back to the console host (galileo-core'sreplace("console", "api")was a no-op) and worked if console and API shared a host. Those deployments now point atapi.<console-host>and stop working untilSPLUNK_AO_API_URLis set.The PR description does call this out under "Compatibility / risk", but the CHANGELOG is what users actually read on upgrade, and it currently reads as a pure improvement. Two other behavior changes are also unrecorded:
otlp_endpointfor a localhost console on a non-8088 port now retargets tohttp://localhost:8088, and hosts containingconsoleas a non-leading substring (customer-console.example.com) now deriveapi.customer-console.example.cominstead ofcustomer-api.example.com.Please add a
### Changedentry with the migration step:🤖 Generated by the Astra agent