Skip to content
This repository has been archived by the owner on Jul 10, 2024. It is now read-only.

OBS-419: Fix distinct ID for Postman API key users. #243

Merged
merged 2 commits into from
Oct 5, 2023
Merged

OBS-419: Fix distinct ID for Postman API key users. #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad84e35
OBS-419: Fix distinct ID for Postman API key users.
mgritter Oct 3, 2023
1e8a8ab
Use Akita API key, not Postman, as fallback
mgritter Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions cfg/credentials.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,24 @@ func WritePostmanAPIKeyAndEnvironment(profile, postmanApiKey, postmanEnvironment

return writeConfigToFile(profile, keyValueMap)
}

// Check whether credentials are present, of any variety.
func CredentialsPresent() bool {
key, _ := GetPostmanAPIKeyAndEnvironment()
if key != "" {
return true
}

key, secret := GetAPIKeyAndSecret()
return key != "" && secret != ""
}

// If we can't call /v1/user to get a distinct ID, we can try using
// the credentials provided -- for now this is just an Akita API Key ID.
// To use a Postman API key we'd have to both obfuscate it (logging
// the user's API key would be bad) and have a way to map it to a
// particular user -- seems better to fall back to local IDs.
func DistinctIDFromCredentials() string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment and the implementation seems contradicting 🤔
We are saying we should not use Postman API Key for distinct id, but we are returning it from this function.

Copy link
Member

@shreys7 shreys7 Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, shouldn't this function try getting an ID from the Akita API Key too if it did not find one from the Postman API Key?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're correct, this was the wrong function.

key, _ := GetAPIKeyAndSecret()
return key
}
10 changes: 6 additions & 4 deletions telemetry/telemetry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,7 @@ func getDistinctID() string {
// If there's no credentials configured, skip the API call and
// do not emit a log message.
// Similarly if telemetry is disabled.
key, secret := cfg.GetAPIKeyAndSecret()
if key != "" && secret != "" && analyticsEnabled {
if cfg.CredentialsPresent() && analyticsEnabled {
// Call the REST API to get the user email associated with the configured
// API key.
ctx, cancel := context.WithTimeout(context.Background(), userAPITimeout)
Expand All @@ -145,8 +144,11 @@ func getDistinctID() string {
printer.Infof("but the agent will still attempt to send telemetry to Postman support.\n")
}

if key != "" {
return key
// Try to derive a distinct ID from the credentials, if present, even
// if the /v1/user call failed.
keyID := cfg.DistinctIDFromCredentials()
if keyID != "" {
return keyID
}

localUser, err := user.Current()
Expand Down