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

fix: ConnectionScopedTelemetry COMPASS-7968 #5960

Closed
wants to merge 18 commits into from
Closed

Conversation

paula-stacho
Copy link
Contributor

@paula-stacho paula-stacho commented Jun 21, 2024

Description

ConnectionScopedTelemetry

Hooks & locator combining telemetry and connectionInfoAccess, by currying the connectionId (from connectionInfoAccess) into track. They live in compass-connections. I'd rather they live in the telemetry, and maybe even be the telemetry default, but right now that would create a circular dependency.

  • useConnectionScopedTelemetry
  • withConnectionScopedTelemetry
  • useConnectionScopedTrackOnChange
  • createConnectionScopedTelemetryLocator

This covers most cases. Occasionally, the event is outside of connectionInfoAccess scope, and we get the connection_id in some other way.

Telemetry strategy

From now on, events & payload have to be registered in @mongodb-js/compass-telemetry/src/events. This is a first small step towards having a clear telemetry strategy in the code. Small step because right now, the payload types are mostly placeholders, I am only requiring connectionId (where possible).
Adding new events, we should define the actual payload type. For the existing events, we can port the current payload types from the code gradually or via some script (or both 😅 ). // TODO: add a task

Checklist

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@@ -209,17 +213,21 @@ export const saveCurrentPipeline =
id: savedPipeline.id,
num_stages: stagesLength,
editor_view_type: mapPipelineModeToEditorViewType(getState()),
connectionId: connectionInfoAccess.getCurrentConnectionInfo().id,
Copy link
Contributor

Choose a reason for hiding this comment

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

connection_id

@paula-stacho paula-stacho changed the title Compass 7968 fix: ConnectionScopedTelemetry COMPASS-7968 Jun 25, 2024
@github-actions github-actions bot added the fix label Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants