Skip to content
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

adding scopes to auth metrics for detailed authStatus #736

Merged
merged 4 commits into from
May 17, 2024

Conversation

dhruvigajjar
Copy link
Contributor

@dhruvigajjar dhruvigajjar commented May 16, 2024

Problem

Adding auth telemetry for to identify user's auth Status upon start up.

Solution

auth_userState list scopes upon users loading extension so we know their authStatus upon startup

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@dhruvigajjar dhruvigajjar requested a review from a team as a code owner May 16, 2024 23:49
Comment on lines 1876 to 1877
"name": "auth_userState",
"description": "The state of the user's authentication.",
Copy link
Contributor

@justinmk3 justinmk3 May 17, 2024

Choose a reason for hiding this comment

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

metrics are for events. state isn't an event

What event does auth_userState represent? It looks more like data. Can the data be stored on an existing event? Otherwise, it's not clear when this "event" should be emitted, it would be more like a heartbeat (which then implies it should be on... an existing, semi-frequent event)

If this is to be done on startup, can it be added to the existing startup event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's only on startup.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes Ideally it should be in common only because JetBrains is also emitting this metric. Trying to fix it by moving it to common.

@dhruvigajjar dhruvigajjar merged commit 7b10365 into main May 17, 2024
8 checks passed
@dhruvigajjar dhruvigajjar deleted the fix/updating-auth_userState-metric branch May 17, 2024 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants