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

Add telemetry with sentry.io #1166

Merged
merged 3 commits into from
Jul 31, 2024

Conversation

rafaellehmkuhl
Copy link
Member

It is opt-out by default, but allows disabling it through the development configuration page.

This package will be used for telemetry gattering.
src/main.ts Outdated
app,
dsn: 'https://5176a29493da4d31a6384c55d291cd3d@o4507692193415168.ingest.us.sentry.io/4507692196036608',
integrations: [Sentry.browserTracingIntegration(), Sentry.replayIntegration()],
tracesSampleRate: 1.0, // Capture 100% of the transactions

Choose a reason for hiding this comment

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

the tracesSampleRate is likely to be the first thing to deplete, and getting 100% does not give much benefit anyway. I´m using it at 0.1 and is already insightful:

image

Choose a reason for hiding this comment

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

there is also the option for sampling the errors ( in python it's usually the exceptions ), might be wise to start with a sampled amount if the app produces exceptions very often

https://docs.sentry.io/platforms/javascript/guides/vue/configuration/options/#sample-rate

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch!
Reduced the trace to 10%.

We are trying not to have unhandled errors, so I think for those it's beneficial to keep 100%.

integrations: [Sentry.browserTracingIntegration(), Sentry.replayIntegration()],
tracesSampleRate: 1.0, // Capture 100% of the transactions
tracePropagationTargets: [],
replaysSessionSampleRate: 0.1, // This sets the sample rate at 10%. You may want to change it to 100% while in development and then sample at a lower rate in production.

Choose a reason for hiding this comment

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

never used the replay options ( only available in paid ) but I think itś supposed to generate a "video-like" session of users using the app, looking forward to see it in action :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm also curious 👀

@voorloopnul
Copy link

Usually depleting the transactions ( tracesSampleRate ) is not a big issue, you just loose performance monitoring, but the errors (sampleRate) should be more carefully managed, specially if we are going to share the account :D

Copy link
Contributor

@ArturoManzoli ArturoManzoli left a comment

Choose a reason for hiding this comment

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

If you change the max-w from 50 to 55 on this line, the switch labels won't have to wrap:
:class="interfaceStore.isOnSmallScreen ? 'max-w-[85vw]' : 'max-w-[50vw]'"

image

image

Copy link
Contributor

@ArturoManzoli ArturoManzoli left a comment

Choose a reason for hiding this comment

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

Everything looks good for me. Maybe we should also wait for @voorloopnul approval before merging.

@rafaellehmkuhl rafaellehmkuhl merged commit 1ebfade into bluerobotics:master Jul 31, 2024
8 checks passed
@rafaellehmkuhl rafaellehmkuhl deleted the add-sentry branch July 31, 2024 14:11
@rafaellehmkuhl rafaellehmkuhl linked an issue Jul 31, 2024 that may be closed by this pull request
@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-needed Change needs to be documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add logs telemetry
4 participants