Skip to content

Add metric aggregation implementation#520

Closed
Nevay wants to merge 1 commit into
open-telemetry:mainfrom
Nevay:feature/metrics
Closed

Add metric aggregation implementation#520
Nevay wants to merge 1 commit into
open-telemetry:mainfrom
Nevay:feature/metrics

Conversation

@Nevay

@Nevay Nevay commented Dec 23, 2021

Copy link
Copy Markdown
Contributor

Adds synchronous and asynchronous metric aggregations with support for multiple readers per aggregation.
Intended as internal implementation of meter instruments; on instrument creation: Meter would create a MetricStream for each registered View, MetricReaders would register as reader for each MetricStream.

Creating this as draft to gather some feedback before I start working on the todos. Are there objections to implementing the metrics SDK this way?


TODOs:

  • remove/resolve compatibility.php
  • add PHP7.4 compatibility (currently ^8.1 only)
  • add tests
  • fix cs / psalm / phpstan / phan

Adds synchronous and asynchronous metric aggregations with support for multiple readers per aggregation.
@tidal

tidal commented Jan 1, 2022

Copy link
Copy Markdown
Member

Creating this as draft to gather some feedback before I start working on the todos. Are there objections to implementing the metrics SDK this way?

I don't see any objections, and honestly the Metrics API/SDK could need some polish. So this is very welcome!

  • add PHP7.4 compatibility (currently ^8.1 only)

Did you take a look into rector?
We used it to upgrade the code-base from PHP 7.3 to 7.4, and there were just minimal (manual) adjustments needed.
I don't know how well it handles downgrading Enums, etc., but I guess it's worth a try.

@bobstrecansky

Copy link
Copy Markdown
Contributor

Hey @Nevay - are you still planning on updating this PR?

@bobstrecansky

Copy link
Copy Markdown
Contributor

Closing this due to inactivity; please feel free to reopen if you'd like to!

@Nevay
Nevay deleted the feature/metrics branch April 19, 2023 19:11
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.

3 participants