Skip to content

sdk/metric: Refactor pipeline and observable measures #5946

Open
@pellared

Description

Do we need to lock around this? It would be good to ensure our parallel tests Invoke a callback in parallel with RegisterCallbacks

Originally posted by @dashpole in #5900 (comment)

I spend literally more than half an hour to figure out the same thing. I think we could refactor the code to make it cleaner or more readable. Maybe we should have a []aggregate.Measure[N] field in type observable[N int64 | float64] struct? Yet, I find it is out of the scope for this PR as I do not think it would be an easy task and having a fix is critical. Making a refactoring with the tests and working functionality would also be an easier task.

However, can we at least add comments that there is no race condition when accessing p.float64Measures and p.int64Measures as the pipeline is already locked by pipeline.Produce? Should we create a follow-up refactoring issue? @pree-dew what are your thoughts here? I do not want to block you.

Side note: I guess it is possible to achieve race conditions, but these would be in cases of bad usage of the Metrics API.

Originally posted by @pellared in #5900 (comment)

Metadata

Assignees

No one assigned

    Labels

    area:metricsPart of OpenTelemetry MetricsenhancementNew feature or requesthelp wantedExtra attention is neededpkg:SDKRelated to an SDK package

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions