Skip to content

Conversation

@mcasimir
Copy link
Collaborator

@mcasimir mcasimir commented Jan 8, 2024

Description

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)

event: 'Telemetry Disabled',
properties: {},
});
void this._flushTelemetryAndIgnoreFailure();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this can't be done anymore without closing the client

Copy link
Contributor

Choose a reason for hiding this comment

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

Just making sure - does this mean that there is a chance that we might lose some telemetry that isn't synced yet? Probably not, right?

Copy link
Collaborator Author

@mcasimir mcasimir Jan 8, 2024

Choose a reason for hiding this comment

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

That should be covered by the other flush, i think the reason this was here before is to "send telemetry one last time" and then stop sending anything after it has been disabled. I don't remember any strict requirement about this, I think that it should be fine to let the flush happen with a bit of delay here as long as we don't queue new events.

event: 'Telemetry Disabled',
properties: {},
});
void this._flushTelemetryAndIgnoreFailure();
Copy link
Contributor

Choose a reason for hiding this comment

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

Just making sure - does this mean that there is a chance that we might lose some telemetry that isn't synced yet? Probably not, right?

@mcasimir mcasimir merged commit ed210a6 into main Jan 9, 2024
@mcasimir mcasimir deleted the update-segment-client branch January 9, 2024 21:21
@gribnoysup gribnoysup changed the title chore(telemetry): update analytics-node to latest version chore(telemetry): update analytics-node to latest version COMPASS-7441 Nov 20, 2024
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.

3 participants