Skip to content

Feature/allow multiple reporters of same type #699

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: features/4.3.0
Choose a base branch
from
Open

Feature/allow multiple reporters of same type #699

wants to merge 2 commits into from

Conversation

@Francois-du-Plessis
Copy link
Contributor

@Francois-du-Plessis Francois-du-Plessis commented Jun 15, 2021 •
edited
Loading

Calling ToInfluxDb multiple times with different filters does not work

Details on the issue fix or feature implementation

  • Removed single reporter per reporter type restriction
  • Updated DefaultReportMetricsRunner to run multiple reporters for a given reporter type

Confirm the following

  • I have ensured that I have merged the latest changes from the dev branch
  • I have successfully run a local build
  • I have included unit tests for the issue/feature
  • I have included the github issue number in my commits

Francois du Plessis added 2 commits June 15, 2021 17:30
AppMetrics#612 Calling ToInfluxDb multiple times with different filte…
81e35a7 
…rs does not work

Altered MetricsReporterCollection to return a Enumaration of reporters of a specific type
Altered MetricsReporterColletion to NOT remove a reporter on addition if the reporter already exists in the collection
Updated tests.
Removed unnecessary return
da02f0a 
Added tests checking multiple reporters.
@alhardy
Copy link
Collaborator

alhardy commented Jun 16, 2021

Could you create the PR on branch features/4.3.0 please. Sorry should make clearer in the readme

@Francois-du-Plessis Francois-du-Plessis changed the base branch from dev to features/4.3.0 June 16, 2021 08:28
@Havret
Copy link

Havret commented Jun 24, 2022

@Francois-du-Plessis, @alhardy Any update on this? It's over a year.

@Francois-du-Plessis
Copy link
Contributor Author

Francois-du-Plessis commented Jun 29, 2022 •
edited
Loading

Hey @Havret, the maintainer is a bit busy with life at the moment. Workaround we are currently using is to wrap the reporter type into a unique type:

/// <summary>
/// Empty wrappper class of InfluxDbMetricsReporter to create a uniquely typed reporter.
/// </summary>
internal class UniqueReporter1InfluxDbReporter : InfluxDbMetricsReporter
{
    internal UniqueReporter1InfluxDbReporter(MetricsReportingInfluxDbOptions options, ILineProtocolClient lineProtocolClient) : base(options, lineProtocolClient)
    {
    }
}

/// <summary>
/// Empty wrappper class of InfluxDbMetricsReporter to create a uniquely typed reporter.
/// </summary>
internal class UniqueReporter2InfluxDbReporter : InfluxDbMetricsReporter
{
    internal UniqueReporter2InfluxDbReporter(MetricsReportingInfluxDbOptions options, ILineProtocolClient lineProtocolClient) : base(options, lineProtocolClient)
    {
    }
}

@Havret
Copy link

Havret commented Jun 29, 2022

Hey @Francois-du-Plessis, thanks for your suggestion. It requires some boilerplate registration code, but works like a charm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Francois-du-Plessis @alhardy @Havret