[CFX-6327] feat(telemetry): send to Amplitude EU ingest - #740
Open
ajalon1 wants to merge 7 commits into
Open
Conversation
|
🎫 Jira: |
Select the Amplitude ServerZone at runtime rather than hard-coding the US ingest endpoint. The zone is inferred from the configured DataRobot endpoint (host contains ".eu." or ends with ".eu" -> EU, else US), with an explicit telemetry-server-zone override taking precedence. The override is settable via the --telemetry-server-zone flag, the DATAROBOT_CLI_TELEMETRY_SERVER_ZONE env var, or the telemetry-server-zone config-file key (case-insensitive US/EU). An invalid value logs a warning to .dr-tui-debug.log and falls back to the inferred zone; telemetry initialization never blocks or errors visibly. The flag is deliberately NOT a universal flag -- telemetry events are emitted by the parent CLI process, not by plugin subprocesses, so forwarding the env var into plugins serves no purpose (decision recorded on CFX-6327; revisit if plugins ever emit their own telemetry). Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Flip --telemetry-server-zone from a non-universal persistent flag to a universal flag so DATAROBOT_CLI_TELEMETRY_SERVER_ZONE is forwarded to plugin subprocesses. Rationale: - Mirrors --disable-telemetry, which is already universal, keeping the telemetry-preference surface uniform. - Non-coercive: plugins that don't emit telemetry ignore it; plugins that do emit their own analytics can honor the user's data-residency preference without a future API change. - Behavioral parity with the Codespace env-injection path (CFX-6328), where the variable is already inherited by every process in the container -- making the flag universal means the --flag path behaves the same as the env-injection path. Revises the earlier "non-universal" decision recorded on CFX-6327. The parent CLI's own zone resolution is unaffected (bindUniversal only controls subprocess env injection). Test guard updated to assert the universal annotation and the TELEMETRY_SERVER_ZONE env-var suffix. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
…8 baseline CFX-7448 removed EU references from the telemetry docs to reflect the state of main (no EU support). This PR re-introduces EU server-zone support, so re-align the docs: - Re-add the api.eu.amplitude.com row to the network-endpoints table (only used when ServerZone is set to EU). - Make the "stored in the USA" statements conditional on the selected server zone, in both docs/development/telemetry.md and docs/user-guide/configuration.md, cross-linking the Server zone / data residency section. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
ajalon1
force-pushed
the
cfx-6327-telemetry-server-zone
branch
from
August 6, 2026 00:13
6fb38b5 to
6fbbba8
Compare
Code OwnershipCli Maintainers
Review requested from the teams above. Labels will be removed automatically upon approval. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit dbee015. Configure here.
…up header Address Cursor Bugbot review findings on internal/telemetry/serverzone.go: 1. (Medium) inferServerZone ran the ".eu" host rules on the full base URL instead of the hostname. GetBaseURL keeps an explicit port, so an EU endpoint with a port (e.g. https://mytenant.eu:8443) failed the HasSuffix(".eu") check and was misclassified as US, routing residency-sensitive telemetry to the wrong Amplitude zone. Now parse the URL and match against u.Hostname() (port-stripped), falling back to the lowercased full string when parsing fails or no host is present. Added table-test cases for EU hosts with explicit ports. 2. (Low) Removed the duplicate Apache license header (introduced by the prior "chore: copyright" commit) so the file carries a single header. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Add explicit inferServerZone table cases for the canonical MTS prod hosts from dr auth set-url / the hostpicker: - app.datarobot.com -> US (already present) - app.eu.datarobot.com -> EU (already present) - app.jp.datarobot.com -> US (new): locks in the intentional Japan routing. Amplitude has no APAC data center, so Japan infers US; the auto-disable follow-up is tracked in CFX-7451. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Contributor
Author
|
/approve-smoke-tests |
Contributor
|
🔐 Fork PR smoke tests triggered by @ajalon1 What happens next:
|
ajalon1
changed the title
Contributor
|
🔐 Fork smoke tests started by maintainer ⏳ Security scans passed. Running smoke tests... Commit: |
Contributor
|
✅ All smoke tests passed! (Fork PR) ✅ Security Scan: success |
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
1 participant
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.
RATIONALE
The CLI telemetry client hard-codes Amplitude's US ingest endpoint. To support EU data-residency requirements without per-region builds, the Amplitude
ServerZonemust be selectable at runtime. CFX-6327 specifies inference from the configureddatarobot_instanceURL with an explicit user-controlled override. The sibling ticket CFX-6328 covers injecting the override env var at Codespace provisioning and depends on this CLI-side support.CHANGES
internal/telemetry/serverzone.go(new):inferServerZone(baseURL)derives US/EU from the DataRobot endpoint (host contains.eu.or ends with.eu→ EU, else US);resolveServerZone()applies an explicittelemetry-server-zoneoverride (flag/env/config) with precedence over inference, case-insensitiveUS/EU, and invalid-value fallback to the inferred zone with a warning.internal/telemetry/telemetry.go:NewClientnow setsconfig.ServerZone = resolveServerZone()and logs the chosen zone at debug level.cmd/root.go: registers the--telemetry-server-zonepersistent root flag and wires it as a universal flag viabindUniversal, soDATAROBOT_CLI_TELEMETRY_SERVER_ZONEis forwarded to plugin subprocesses. This mirrors--disable-telemetryand keeps behavior consistent with the Codespace env-injection path (CFX-6328), where the variable is already inherited by every process. Plugins that don't emit telemetry simply ignore it.internal/config/constants.go: adds the sharedTelemetryServerZoneconfig-key constant.config.PersistableKeys: follows thedisable-telemetryprecedent — bound to viper but not written back todrconfig.yaml.Env var / flag / config surface
dr --telemetry-server-zone EU <command>DATAROBOT_CLI_TELEMETRY_SERVER_ZONE=EUtelemetry-server-zone: EUindrconfig.yamlNOTES
--telemetry-server-zoneis wired throughbindUniversal, soDATAROBOT_CLI_TELEMETRY_SERVER_ZONEis injected into plugin subprocess environments. Rationale: mirrors--disable-telemetry, is non-coercive (plugins that don't emit telemetry ignore it), is forward-compatible for plugins that do emit analytics, and gives behavioral parity with the Codespace env-injection path (CFX-6328) where the variable is already inherited by all processes. (Revised from an earlier "non-universal" decision — see the updated comment on CFX-6327.)disable-telemetry: true;telemetry-server-zonedoes not accept an APAC value (invalid values warn and fall back to inference).ServerURL(the SDK's lower-level endpoint override) is intentionally not exposed.US/EUlogs a warning to.dr-tui-debug.logand falls back to the inferred zone; telemetry init never blocks or errors visibly.TESTING
internal/telemetry/serverzone_test.go: covers inference (incl..europefalse-positive guard, case-insensitive host), override-wins-over-inference, case-insensitive override, invalid-value fallback to inferred, and whitespace-as-unset.cmd/root_test.go::TestTelemetryServerZoneFlagRegistered: guards that the flag is registered, IS marked universal, and maps to theTELEMETRY_SERVER_ZONEenv-var suffix.go test -race ./internal/telemetry/... ./cmd— pass.task lint— 0 issues.go run . --helpconfirms--telemetry-server-zoneis registered and visible.RELATED
Note
Medium Risk
Changes where usage analytics are sent (data residency) and adds a new universal env var for plugins; behavior is designed to fail safe with inference fallback and no user-visible errors on bad values.
Overview
Adds runtime Amplitude ingest region (US vs EU) so telemetry can meet EU data-residency without separate builds.
--telemetry-server-zoneis a new persistent root flag (plusDATAROBOT_CLI_TELEMETRY_SERVER_ZONE/telemetry-server-zonein config). It is a universal flag, forwarded to plugins like--disable-telemetry.NewClientnow sets the Amplitude SDKServerZoneviaresolveServerZone(): explicitUS/EUwins over inference from the DataRobot endpoint (.eu.in host or.eusuffix → EU); invalid values warn and fall back without failing the CLI.Docs cover EU endpoint allowlisting and user configuration. Tests cover inference, override precedence, and flag registration.
Reviewed by Cursor Bugbot for commit dbee015. Configure here.