Skip to content
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

[API Proposal]: Allow to delete an instrument inside a Meter instance #83822

Open
LGouellec opened this issue Mar 23, 2023 · 11 comments
Open

[API Proposal]: Allow to delete an instrument inside a Meter instance #83822

LGouellec opened this issue Mar 23, 2023 · 11 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Diagnostics.Metric enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@LGouellec
Copy link

Background and motivation

For now, when you create an instance of Meter you can create multiple types of instrument (Histogram, Counter, ObservableGauge or ObservableCounter).

But you can't delete one observable gauge (for instance) previously created. The only way is to dispose the Meter instance and recreate from scratch with all instruments minus the instrument supposed to remove. It's not efficient at all.

API Proposal

 public class Meter : IDisposable
{
       private List<Instrument> _instruments = new List<Instrument>();
       // ....

      public void Remove(Instrument instrument) {
           _instruments.Remove(instrument)
     }
}

API Usage

var meter = new Meter("ServiceName");
var gauge =  meter.CreateObservableGauge(
                        key, 
                        () => new[]
                        {
                            new Measurement<double>(
                                value,
                                tags)
                        },
                        description: summary);           

meter.Remove(gauge);

Alternative Designs

No response

Risks

No response

@LGouellec LGouellec added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 23, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Mar 23, 2023
@tommcdon
Copy link
Member

@noahfalk @tarekgh

@tommcdon tommcdon added this to the 8.0.0 milestone Mar 27, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Mar 27, 2023
@tommcdon tommcdon added enhancement Product code improvement that does NOT require public API changes/additions untriaged New issue has not been triaged by the area owner and removed untriaged New issue has not been triaged by the area owner labels Mar 27, 2023
@tarekgh tarekgh modified the milestones: 8.0.0, Future Mar 27, 2023
@asasine
Copy link

asasine commented Sep 20, 2023

I would think that having Instrument implement IDisposable is the canonical way to indicate that instruments are disposable. Stealing from the Observer design pattern, the instances of Instrument can hold references to their Meter and remove themself from the meter's collection when disposed. This further supports the DI model as types only need to depend on the appropriate instrument, not the meter, to accurately dispose of the instrument when the DI container disposes them.

@LGouellec
Copy link
Author

Hey there,

Do you have any news on top of that matter ?

Best regards,

@julealgon
Copy link

@LGouellec could you elaborate what kinds of use cases this would be used with? I tend to agree with your proposal but I'm curious to understand what scenario you have that requires removing instruments dynamically like this.

@LGouellec
Copy link
Author

@julealgon

I work on this open source project , a .Net Kafka Streams implementation.
Typically, I need to expose couple of instruments based on a Kafka Partition and probably you know but the kafka partitions can be reassigned if you have more consumers in the loop, basically if you scale in or out the number of your applications.

So when the partition is reassigned, I need to drop the instrument in the app A to avoid incoherent monitoring.

Let me know, if you have any further questions.

@BrannanKovachev
Copy link

My team is also interested in such a feature. We would like our application to begin collecting specific metrics from a gauge given a condition. Then once the condition is no longer met, stop the instrument from reporting. It doesn't make sense to destroy the entire meter and re-register all the instruments.

The OpenTelemetery Metrics API docs describe this as a requirement (you can look at any of the asynchronous instruments).

@julealgon
Copy link

My team is also interested in such a feature.

@BrannanKovachev upvote the original post to signal that if you can.

@tmakin
Copy link

tmakin commented Oct 9, 2024

I've been exploring the new Meter features with a view to replacing the Prometheus.net exporter. The lack of remove/delete for expired instruments is currently a blocker for adoption, so I think this feature would be really useful. My use case is to track equipment with intermittent availability, when the source goes offline I don't want the old metrics hanging around. Tearing down the entire meter every few seconds seems like overkill, as there are some instruments that I don't want to drop.

@LGouellec
Copy link
Author

For you information, the way that we can't delete a specific metrics force me to Dispose the Meter after each iteration which generate a MemoryLeak in the OpenTelemetry reporter. See : open-telemetry/opentelemetry-dotnet#5922

@LGouellec
Copy link
Author

@julealgon

Any change that it will be part of 10.0 milestone ?

@julealgon
Copy link

@julealgon

Any change that it will be part of 10.0 milestone ?

I can't answer to that as I'm not part of the .NET team @LGouellec , but I hope it does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Diagnostics.Metric enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

8 participants