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

DefaultMeterFactory does not pass MeterOptions future properties to Meter constructor #97941

Closed
KalleOlaviNiemitalo opened this issue Feb 4, 2024 · 0 comments · Fixed by #103791
Assignees
Labels
area-System.Diagnostics.Metric in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Feb 4, 2024

Description

If more properties are added to System.Diagnostics.Metrics.MeterOptions in the future, then Microsoft.Extensions.Diagnostics.Metrics.DefaultMeterFactory should pass them too to the System.Diagnostics.Metrics.Meter constructor. However, the DefaultMeterFactory implementation in .NET 8.0.1 passes only those properties that it knows about: Name, Version, Tags, and a fixed Scope.

This was noted in #86740 (comment), and the implementation was then changed, but the change did not fix it. The code in .NET 8.0.1 is:

object? scope = options.Scope;
options.Scope = this;
FactoryMeter m = new FactoryMeter(options.Name, options.Version, options.Tags, scope: this);
options.Scope = scope;

It changes and restores MeterOptions.Scope but does not pass the MeterOptions reference to the FactoryMeter constructor. Thus:

  • Nothing reads the temporary value from MeterOptions.Scope, i.e. the assignments are pointless.
  • If a future version of .NET has extra properties in MeterOptions, then DefaultMeterFactory won't pass them to FactoryMeter, and FactoryMeter won't pass them to Meter, which will instead use default values.

Reproduction Steps

Cannot be reproduced before more properties are added to MeterOptions.

Expected behavior

DefaultMeterFactory should pass through the MeterOptions reference all the way to Meter.

Actual behavior

DefaultMeterFactory passes through only the known properties of MeterOptions to Meter.

Regression?

Not a regression.

Known Workarounds

Register a custom IMeterFactory implementation in the dependency injection container.

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 4, 2024
@tarekgh tarekgh self-assigned this Jun 20, 2024
@tarekgh tarekgh added this to the 9.0.0 milestone Jun 20, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Jun 20, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Jun 21, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Diagnostics.Metric in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants