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: refactor telemetry to use a more standardized format VSCODE-651 #891

Open
wants to merge 6 commits into
base: gagik/add-playground-buttons
Choose a base branch
from

Conversation

gagik
Copy link
Contributor

@gagik gagik commented Nov 29, 2024

Built on top of #890

Makes some minor quality of life improvements to our current telemetry setup, especially for sendMessageToParticipant.

  1. Introduce a telemetry field in sendMessageToParticipant (ideally a structure we could use in other places in the future?), which is meant to be a scalable structure for us to pass different arguments to functions we want to track without messing with our objects and too much manual work.
  2. Make command field in various telemetry functions stricter; introduce and cleanup some types to better fit this. Right now we are not keeping a good track of what we call a command when sending telemetry, so this is hoping to document this better with types.
  3. Remove Copilot prefix in places where it may be redundant or not match its property type.

Let me know what you think; this was mainly me a result of me trying to understand & document our current setup better, would be happy to adjust accordingly 🙂

@gagik gagik changed the base branch from main to gagik/add-playground-buttons November 29, 2024 14:09
const commandPrefix = command ? `/${command} ` : '';
const query = `@MongoDB ${commandPrefix}${message}`;

if (telemetry) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an example of it might be nice to structure tracking in other functions as some hot-pluggable thing.

@@ -477,14 +517,14 @@ export default class TelemetryService {
command,
error_code: errorCode,
error_name: errorName,
});
} as ParticipantResponseFailedProperties);
Copy link
Contributor

Choose a reason for hiding this comment

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

[non-blocking]

Suggested change
} as ParticipantResponseFailedProperties);
} satisfies ParticipantResponseFailedProperties);

I think we can do this now, our TS version and tooling seems to be fine with it (and it's generally nicer since it avoids the behavior of as that allows type coercions even in case of mismatches)

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.

2 participants