Skip to content

chore: enable metrics by default#1609

Draft
JoshuaMoelans wants to merge 5 commits intomasterfrom
joshua/chore/set_default_metrics_true
Draft

chore: enable metrics by default#1609
JoshuaMoelans wants to merge 5 commits intomasterfrom
joshua/chore/set_default_metrics_true

Conversation

@JoshuaMoelans
Copy link
Copy Markdown
Member

@JoshuaMoelans JoshuaMoelans commented Mar 30, 2026

Metrics captured by the Public API should be opt-out as per develop docs.

Since we don't have any internal auto-instrumented metrics, we enable it by default so calling sentry_metrics_X will just capture instead of requiring setting the option. ⚠️ this does make it so our batching thread starts regardless of whether it actually has any work (see the discussion below).

Also updates the tests to reflect this change; by default it is true, but one can set it to false with sentry_options_set_enable_metrics(options, false); (for example, when only wanting metrics for debug and not production).

follow-up


Discussion

CI currently fails because we now log from our Metrics Batcher Thread that starts by default; we can fix this either by

a) changing those logging tests to expect the output (but then starting the metrics batcher even if no metric is ever sent) #1612
b) adding a lazy init to the metrics/logs batcher #1611

  • think about whether we also want the metrics batcher to always be running, since enable_metrics just starts it; maybe we only want to do so once the first metric is captured? ⚠️ currently, if we don't do this, this changes the behavior of the SDK (a batching thread starts even if metrics wasn't explicitly enabled, since it is now on by default)

@JoshuaMoelans JoshuaMoelans requested a review from mujacica March 30, 2026 11:54
@JoshuaMoelans
Copy link
Copy Markdown
Member Author

CI currently fails because we now log from our Metrics Batcher that starts by default; we can fix this either by

a) changing those logging tests to expect the output (but then starting the metrics batcher even if no metric is ever sent)
b) adding a lazy init to the metrics/logs batcher #1611

@JoshuaMoelans JoshuaMoelans marked this pull request as draft March 30, 2026 14:51
@JoshuaMoelans JoshuaMoelans changed the title chore: enable metrics by defaullt chore: enable metrics by default Mar 31, 2026
JoshuaMoelans and others added 3 commits March 31, 2026 16:17
* update test to expect batching thread log message

* fix(tests): disable metrics in logger unit tests to avoid tsan race

The batcher thread spawned by metrics startup logs via the custom
logger callback, racing with the main thread's writes to assert_now.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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