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

feat(amazonq): Add credential start url for amazonq events #701

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

leigaol
Copy link
Contributor

@leigaol leigaol commented Feb 19, 2024

Problem

Amazon Q telemetry events should have startUrl in its field. Because there is a need for more granularity in user connections in order to help diagnose problems.

Solution

Add credential start url for amazonq events. Option for backward compatibility.

License

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

@leigaol leigaol requested a review from a team as a code owner February 19, 2024 22:06
@@ -2791,7 +2791,8 @@
"description": "Captures end of the conversation with amazonq /dev",
"metadata": [
{ "type": "amazonqConversationId" },
{ "type": "amazonqEndOfTheConversationLatency", "required": false }
{ "type": "amazonqEndOfTheConversationLatency", "required": false },
{ "type": "credentialStartUrl", "required": false }
]
},
Copy link
Contributor

Choose a reason for hiding this comment

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

will all amazon Q metrics will have credentialStartUrl? or is it like after auth, all metrics will have.
In which cases credentialStartUrl will be empty?

Copy link
Contributor Author

@leigaol leigaol Feb 19, 2024

Choose a reason for hiding this comment

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

After auth, all metrics will have credentialStartUrl. As long as you are signed in, you will have a valid non empty credentialStartUrl

Copy link
Contributor

Choose a reason for hiding this comment

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

so for users who's auth is expired, we wont have startUrl or we will have last valid startUrl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. It is the current active start url.
  2. If auth expired, there won't be any startUrl. User with expired auth won't be able to invoke the Code path in Q that sends out any telemetry. Without a valid auth, they won't be able to interact with this product at all. Without any valid user interaction, there won't be any telemetry data.

Copy link
Contributor

Choose a reason for hiding this comment

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

So which metrics do you emit without auth?
Does that mean some metric must have startUrl because they wont be emitted if auth is invalid?

@justinmk3
Copy link
Contributor

merge conflict

@leigaol
Copy link
Contributor Author

leigaol commented Feb 20, 2024

merge conflict

Fixed in latest commit

@nkomonen-amazon nkomonen-amazon merged commit 2b009d8 into aws:main Feb 20, 2024
7 checks passed
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.

5 participants