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

Conversation

mgritter
Copy link
Contributor

@mgritter mgritter commented Oct 3, 2023

Previously we were calling /v1/users to get an email to use as a distinct ID only when an Akita API Key was present.

Previously we were calling /v1/users to get an email to use as a distinct
ID only when an Akita API Key was present.
@mgritter mgritter added the 3 – Normal Priority Non-blocking review—please turn around quickly label Oct 3, 2023
// 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

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.

@mgritter mgritter requested a review from shreys7 October 4, 2023 19:41
@mgritter mgritter merged commit 7435d8c into main Oct 5, 2023
@mgritter mgritter deleted the mgritter/user_telemetry_for_postman_keys branch October 5, 2023 22:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3 – Normal Priority Non-blocking review—please turn around quickly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants