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.

Sign up for GitHub

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

Jump to bottom

(spike) added user to rum #28214

Merged
merged 12 commits into from Mar 11, 2024
Merged

(spike) added user to rum #28214

merged 12 commits into from Mar 11, 2024

Conversation

mdewey
Copy link
Contributor

@mdewey mdewey commented Feb 27, 2024 •
edited

Summary

We are looking for a way to track unique users through the system for RUM purposes. This is a spike PR to test this in staging.

Related issue(s)

No related issue, just slack threads.

Testing done

  • CI passes
  • Need to test in staging.

Screenshots

No UI changes

What areas of the site does it impact?

Secure Message

Acceptance criteria

Quality Assurance & Testing

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable). N/A
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Linting warnings have been addressed
  • Documentation has been updated (link to documentation *if necessary)
  • A screenshot of the developed feature is added. N/A
  • Accessibility testing has been performed N/A

Error Handling

  • The Browser console contains no warnings or errors.
  • Events are being sent to the appropriate logging solution
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)

Authentication

  • Did you login to a local build and verify all authenticated routes work as expected with a test user. N/A

@va-vfs-bot va-vfs-bot temporarily deployed to master/feature/user-rum-session/main February 27, 2024 20:56 Inactive
acrollet
acrollet previously approved these changes Feb 27, 2024
View reviewed changes
src/applications/mhv/shared/hooks/useDatadogRum.jsx Outdated Show resolved Hide resolved
src/applications/mhv/shared/hooks/useDatadogRum.jsx Outdated Show resolved Hide resolved
@va-vfs-bot va-vfs-bot temporarily deployed to master/feature/user-rum-session/main February 27, 2024 23:34 Inactive
mdewey
mdewey commented Mar 4, 2024
View reviewed changes
src/applications/mhv/shared/hooks/useDatadogRum.jsx Outdated
useEffect(
() => {
initializeDatadogRum(config);
if (userInfo.loggedIn && environment.isStaging()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: if this works in, staging. We can explore adding a feature flag vs just turning it on.

@mdewey mdewey marked this pull request as ready for review March 4, 2024 17:08
@mdewey mdewey requested review from a team as code owners March 4, 2024 17:08
acrollet
acrollet previously approved these changes Mar 4, 2024
View reviewed changes
Copy link
Contributor

@acrollet acrollet left a comment

Choose a reason for hiding this comment

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

re-approving 🚀

@va-vfs-bot va-vfs-bot temporarily deployed to master/feature/user-rum-session/main March 4, 2024 17:34 Inactive
micahchiang
micahchiang previously approved these changes Mar 4, 2024
View reviewed changes
@wafimohamed wafimohamed dismissed stale reviews from micahchiang and acrollet via 041e1ec March 4, 2024 21:22
@va-vfs-bot va-vfs-bot temporarily deployed to master/feature/user-rum-session/main March 4, 2024 21:23 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/feature/user-rum-session/main March 4, 2024 23:17 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/feature/user-rum-session/main March 4, 2024 23:50 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/feature/user-rum-session/main March 5, 2024 14:05 Inactive
acrollet
acrollet previously approved these changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@acrollet acrollet left a comment

Choose a reason for hiding this comment

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

re-approving

oleksii-morgun
oleksii-morgun previously approved these changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@oleksii-morgun oleksii-morgun left a comment

Choose a reason for hiding this comment

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

great addition 👍🏻

radavis
radavis previously approved these changes Mar 6, 2024
View reviewed changes
Copy link
Contributor

@radavis radavis left a comment

Choose a reason for hiding this comment

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

Looks good. Left a suggestion.

src/applications/mhv/secure-messaging/containers/App.jsx Outdated Show resolved Hide resolved
@mdewey mdewey dismissed stale reviews from radavis and oleksii-morgun via a37d78d March 6, 2024 16:07
@mdewey mdewey requested review from a team as code owners March 6, 2024 16:07
@va-vfs-bot va-vfs-bot temporarily deployed to master/feature/user-rum-session/main March 6, 2024 16:12 Inactive
@mdewey mdewey removed request for a team March 6, 2024 16:39
@va-vfs-bot va-vfs-bot temporarily deployed to master/feature/user-rum-session/main March 6, 2024 16:51 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/feature/user-rum-session/main March 6, 2024 19:23 Inactive
@mdewey mdewey removed the request for review from a team March 11, 2024 15:56
acrollet
acrollet approved these changes Mar 11, 2024
View reviewed changes
Copy link
Contributor

@acrollet acrollet left a comment

Choose a reason for hiding this comment

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

🐙

oleksii-morgun
oleksii-morgun approved these changes Mar 11, 2024
View reviewed changes
Copy link
Contributor

@oleksii-morgun oleksii-morgun left a comment

Choose a reason for hiding this comment

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

re-approving

@mdewey mdewey merged commit 0e1b4a7 into main Mar 11, 2024
81 of 82 checks passed
@mdewey mdewey deleted the feature/user-rum-session branch March 11, 2024 17:55
pjhill pushed a commit that referenced this pull request Mar 14, 2024
@mdewey @acrollet
(spike) added user to rum (#28214)
ae9d616 
* (spike) added user to rum

* (feature) refined user tracking

* Fix unit test

* (refactor) moved to the correct hook

* (feedback)

---------

Co-authored-by: Adrian Rollett <acrollet@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acrollet acrollet acrollet approved these changes

@radavis radavis radavis approved these changes

@oleksii-morgun oleksii-morgun oleksii-morgun approved these changes

@micahchiang micahchiang micahchiang left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@mdewey @acrollet @radavis @micahchiang @oleksii-morgun @va-vfs-bot