Skip to content

Conversation

mikeland73
Copy link
Contributor

Summary

This PR improves segment logging by:

  • Adding orgID if user is logged in.
  • Using userID if user is logged in.
  • Removing anonymousID if userID is non zero. (Segment will remove userID if we pass an anonymousID)
  • Add new event to track nix build

This allows us to:

  • Use orgID to segment data, potentially allowing individual orgs to export/use their own data as needed.
  • Track logged in users.
  • Track which packages are slow to build.

Private Notes:

  • We still respect DO_NOT_TRACK even if user is logged in.
  • userID and orgID data is only sent if user is logged in.

cc: @Lagoja

How was it tested?

Logged to development segment/amplitude, built a quick table showing nix average build times:

image

@mikeland73 mikeland73 requested review from gcurtis and savil April 24, 2024 04:45
Copy link
Collaborator

@gcurtis gcurtis left a comment

Choose a reason for hiding this comment

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

Is there a way to get the current username without refreshing the token? I think that would also let us remove the context from the telemetry functions, which should probably still run even if the context is cancelled (to report things like timeouts).

}

func userID(ctx context.Context) string {
tok, err := identity.Get().GenSession(ctx)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way to never refresh the token here and in orgID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, our auth library has a GetSessions which returns refreshableTokenSource which have a Peek() function. I'll switch to that.

@mikeland73 mikeland73 merged commit 18eca38 into main Apr 24, 2024
@mikeland73 mikeland73 deleted the landau/better-metrics branch April 24, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants