Skip to content

Conversation

@Litarnus
Copy link
Contributor

@Litarnus Litarnus commented Nov 19, 2025

Introducing metrics for PHP!

Metrics have 3 supported types

  • Counter
  • Gauge
  • Distribution

Each of those metrics can be submitted by calling
\Sentry\metrics()->count('test-counter', 1, ['tag' => 'foo'])

It's also possible to attach units:
\Sentry\metrics()->gauge('test-gauge', 10, ['tag' => 'foo'], \Sentry\Metrics\Unit::second())

Up to 1000 metrics can be stored per request. Older metrics are dropped when the limit is reached.

Metrics need to be flushed manually by calling \Sentry\metrics()->flush().

@Litarnus Litarnus marked this pull request as ready for review November 19, 2025 10:54
Copy link
Collaborator

@stayallive stayallive left a comment

Choose a reason for hiding this comment

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

Code wise the looks good (especially much love for the RingBuffer).

The only thing I'm worried about a bit is that we modify the existing code, namely MetricsUnit and some parameters which is a backwards incompatible break for people that did not yet cleanup their code after we deprecated the previous metrics iteration. Do we feel like that is an acceptable risk? If this is going in 5.x this is a non-issue but the target is master so hence my ask.

@Litarnus
Copy link
Contributor Author

Code wise the looks good (especially much love for the RingBuffer).

The only thing I'm worried about a bit is that we modify the existing code, namely MetricsUnit and some parameters which is a backwards incompatible break for people that did not yet cleanup their code after we deprecated the previous metrics iteration. Do we feel like that is an acceptable risk? If this is going in 5.x this is a non-issue but the target is master so hence my ask.

Initially we wanted to take the risk but after a longer thought we decided against it. I will revert the Metrics class back to how it originally was and will introduce a TraceMetrics class with the new methods.
I will also add a new alias for Unit -> MetricsUnit since we want to use the unit in different places too.

Thanks for your review, I make this a draft again and ping you once it's updated!

@Litarnus Litarnus marked this pull request as draft November 24, 2025 09:17
@Litarnus Litarnus marked this pull request as ready for review November 25, 2025 16:11
@Litarnus Litarnus requested a review from stayallive November 25, 2025 16:11
string $key,
$value,
MetricsUnit $unit,
Unit $unit,
Copy link
Member

Choose a reason for hiding this comment

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

You could leave this as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PHPStan doesn't seem to properly resolve aliases so it will complain. But this should work nonetheless since PHP will resolve the alias and treat Unit and MetricsUnit as equivalent.
To satisfy PHPStan we would either have to ignore the error message or subclass it

Copy link
Collaborator

@stayallive stayallive left a comment

Choose a reason for hiding this comment

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

Code wise this looks good to me, I haven't dug into the spec much yet but I assume you and Michi got that covered pretty well already!

@Litarnus Litarnus merged commit dc31502 into master Nov 27, 2025
44 checks passed
@Litarnus Litarnus deleted the metrics branch November 27, 2025 14:23
@Litarnus Litarnus linked an issue Nov 27, 2025 that may be closed by this pull request
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.

Sentry Metrics for PHP

4 participants