Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Svelte: Add telemetry v2 to svelte client#63041

Merged
vovakulikov merged 3 commits into
mainfrom
vk/add-telemetry-v2-to-svelte-client
Jun 4, 2024
Merged

Svelte: Add telemetry v2 to svelte client#63041
vovakulikov merged 3 commits into
mainfrom
vk/add-telemetry-v2-to-svelte-client

Conversation

@vovakulikov

@vovakulikov vovakulikov commented Jun 3, 2024

Copy link
Copy Markdown
Contributor

Completes SRCH-68

This PR simply adds yet another telemetry v2 usage to the svelte FE client.

Test plan

Manual testing of newly added events

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dadlerj I changed this to 'server.svelte-web' to be able select/filter out only svelte
events later on our dashboards

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this part of the client name? Aren't all of our events attached to an instance anyways?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@camdencheek are you asking about dotcom and server.<instance-name> naming?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm asking why our client name has to include whether or not we're on dotcom. I think our client name should just be svelte-web, because that's what is sourcing these events. Whether or not the instance is dotcom should be determined by the instance ID attached to the event logs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(also, if we can get rid of one more thing that is special-cased for dotcom, let's do it)

@vovakulikov vovakulikov force-pushed the vk/add-telemetry-v2-to-svelte-client branch from 88a275a to cd34f60 Compare June 3, 2024 15:37
Comment thread client/web-sveltekit/src/routes/search/SearchResults.svelte
@vovakulikov vovakulikov merged commit 5915788 into main Jun 4, 2024
@vovakulikov vovakulikov deleted the vk/add-telemetry-v2-to-svelte-client branch June 4, 2024 09:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants