Conversation
cdevent
changed the title
ajalon1
left a comment
ajalon1
left a comment
There was a problem hiding this comment.
LGTM, maybe some more tests around aliases.
| return err | ||
| } | ||
|
|
||
| config.SetAPIConsumerTrace(config.CommandPathToTrace(cmd.CommandPath())) |
There was a problem hiding this comment.
Nice, I like that cmd.CommandPath() is a thing.
| if err := initializeConfig(cmd); err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
I'd add a small docstring explaining what exactly is happening here, but that's fine
|
|
||
| # API consumer tracking (default: true) | ||
| # Set to false to disable the X-DataRobot-Api-Consumer-Trace header on API requests. | ||
| # Matches the Python SDK's DATAROBOT_API_CONSUMER_TRACKING_ENABLED behavior. | ||
| # When enabled, the header value identifies the command being run using dot-notation: | ||
| # datarobot.cli.<command>.<subcommand> (e.g. datarobot.cli.templates.setup) | ||
| export DATAROBOT_API_CONSUMER_TRACKING_ENABLED=false |
There was a problem hiding this comment.
Once we add in the rest of telemetry we should probably explain this further.
| // CommandPathToTrace converts a cobra command path (e.g. "dr templates setup") | ||
| // to the canonical dot-notation trace format (e.g. "datarobot.cli.templates.setup"). | ||
| func CommandPathToTrace(commandPath string) string { | ||
| parts := strings.Fields(commandPath) | ||
| if len(parts) == 0 { | ||
| return "datarobot.cli" | ||
| } | ||
|
|
||
| parts[0] = "datarobot.cli" | ||
|
|
||
| return strings.Join(parts, ".") | ||
| } |
There was a problem hiding this comment.
Checking a few things here -- Cobra doesn't appear to allow dots or periods in command names, which tracks.
Also, I wonder how this works with command aliases, like dr r for dr run, as well with shell aliases, like datarobot start for dr start. I assume that CommandPath() provides a canonical name, based off of:
// CommandPath returns the full path to this command.
func (c *Command) CommandPath() string {
if c.HasParent() {
return c.Parent().CommandPath() + " " + c.Name()
}
return c.DisplayName()
}
There was a problem hiding this comment.
hmm, not sure what to say here ... need to check that.. let's do that on next steps
cdevent
force-pushed
the
cdevent/CFX-5233
branch
from
2aeb337 to
b417cec
Compare
2 participants
This pull request introduces API consumer tracking to the CLI, enabling the inclusion of a
X-DataRobot-Api-Consumer-Traceheader in API requests. This header identifies the running command in dot-notation format (e.g.,datarobot.cli.templates.setup), aligning with the Python SDK's behavior. The feature is enabled by default and can be disabled via an environment variable. The implementation includes configuration changes, header injection in API requests, utility functions, and comprehensive tests.API Consumer Tracking Feature:
Added support for the
X-DataRobot-Api-Consumer-Traceheader in API requests, using a dot-notation command path to identify the CLI command being executed (e.g.,datarobot.cli.templates.setup). This is controlled by theDATAROBOT_API_CONSUMER_TRACKING_ENABLEDenvironment variable, which is enabled by default to match the Python SDK convention. (cmd/root.go[1] [2];internal/config/constants.go[3];docs/user-guide/configuration.md[4]Implemented utility functions in
internal/config/api.gofor managing the trace value:SetAPIConsumerTrace,GetAPIConsumerTrace,CommandPathToTrace, andIsAPIConsumerTrackingEnabled. These handle storing, formatting, and retrieving the trace value and checking if tracking is enabled. (internal/config/api.gointernal/config/api.goR72-R111)Integration with API Requests:
X-DataRobot-Api-Consumer-Traceheader into outgoing API requests in both the authentication (internal/config/auth.go) and general API (internal/drapi/get.go) code paths, using the appropriate trace value. (internal/config/auth.go[1];internal/drapi/get.go[2]Testing:
internal/config/api_test.gointernal/config/api_test.goR110-R171)RATIONALE
CHANGES
PR Automation
Comment-Commands: Trigger CI by commenting on the PR:
/trigger-smoke-testor/trigger-test-smoke- Run smoke tests/trigger-install-testor/trigger-test-install- Run installation testsLabels: Apply labels to trigger workflows:
run-smoke-testsorgo- Run smoke tests on demand (only works for non-forked PRs)Important
For Forked PRs: If you're an external contributor, the
run-smoke-testslabel won't work. A maintainer must manually trigger the "Fork PR Smoke Tests" workflow from the Actions tab, providing your PR number. Please comment requesting a maintainer review if you need smoke tests to run.Note
Medium Risk
Behavior change adds a new request header on all CLI API calls by default, which could affect downstream logging/routing and needs validation across endpoints. Scope is limited to header construction/config and is easy to disable via env var if issues arise.
Overview
Adds API consumer tracking to the CLI by generating a dot-notation trace from the current cobra command path, storing it at startup, and attaching it as
X-DataRobot-Api-Consumer-Traceon outgoing requests (including auth token verification).Tracking is enabled by default via new config key
api-consumer-tracking-enabledand can be disabled withDATAROBOT_API_CONSUMER_TRACKING_ENABLED=false; docs and unit tests were added for the trace formatting, storage, and enablement toggle.Reviewed by Cursor Bugbot for commit b417cec. Bugbot is set up for automated code reviews on this repo. Configure here.