-
-
Notifications
You must be signed in to change notification settings - Fork 199
WIP: feat: metrics #1498
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
Draft
jpnurmi
wants to merge
13
commits into
master
Choose a base branch
from
jpnurmi/feat/metrics
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
WIP: feat: metrics #1498
+1,572
−61
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
422c6ad to
875c3d9
Compare
Per the Sentry metrics specification, counter metrics should use 64-bit signed integers. This is consistent with sentry_value_new_int64 / sentry_value_as_int64 in sentry.h. Also refactored internal record_metric to pass sentry_value_t instead of double, allowing proper type preservation. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
The value parameter was not being decremented when metrics were disabled, causing a memory leak. Co-Authored-By: Claude Opus 4.5 <[email protected]>
SDKs "should offer constants or similar that help customers send in units we support" as specified in the developer docs. See: - https://develop.sentry.dev/sdk/telemetry/metrics/ - https://develop.sentry.dev/sdk/telemetry/attributes/#units Co-Authored-By: Claude Opus 4.5 <[email protected]>
Per the developer docs, units are only used for distribution and gauge metrics, not counters. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Split blocking shutdown and force_flush operations into begin (non-blocking trigger) and wait (blocking completion) phases. This allows logs and metrics operations to run in parallel in sentry_flush() and sentry_close(). Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add metrics integration tests to match the existing logs test coverage: - test_metrics_threaded: concurrent metrics from 50 threads - test_metrics_global_and_local_attributes_merge: global + local attributes - test_metrics_discarded_on_crash_no_backend: metrics discarded without backend - test_metrics_on_crash: parameterized test for inproc/breakpad backends Also refactor thread creation in example.c into a shared run_threads() helper function used by both logs-threads and metrics-threads commands. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Allow reusing attribute objects across multiple log calls, as documented. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Global attributes set via sentry_set_attribute() were not being merged into metrics. Now scope attributes are merged with user attributes having priority on conflicts. Co-Authored-By: Claude Opus 4.5 <[email protected]>
48664ff to
725b062
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Implements the Sentry Metrics feature for the native SDK, allowing applications to record counter, gauge, and distribution metrics that are batched and sent to Sentry: https://develop.sentry.dev/sdk/telemetry/metrics/.
Closes #1453
Features
before_send_metriccallback for filtering or modifying metrics before sendingSENTRY_UNIT_*constants for common telemetry units (byte, kilobyte, millisecond, second, etc.)sentry_flush()andsentry_close()Public API
Considerations
The current API requires passing
sentry_value_new_null()when no custom attributes are needed. An alternative would be to provide simplified variants without the attributes parameter (e.g.,sentry_metrics_count()) alongside explicit variants (e.g.,sentry_metrics_count_with_attributes()). Feedback welcome on which approach is preferred.Dependenciesref(logs): extract reusable batcher for metrics #1495ref(logs): extract attribute helpers for metrics #1497Test plan
tests/unit/test_metrics.c)tests/test_integration_http.py)🤖 Generated with Claude Code