Skip to content

[HUDI-5847] Add support for multiple metric reporters and metric labels#8041

Merged
codope merged 7 commits intoapache:masterfrom
lokeshj1703:multipleReporters
Mar 3, 2023
Merged

[HUDI-5847] Add support for multiple metric reporters and metric labels#8041
codope merged 7 commits intoapache:masterfrom
lokeshj1703:multipleReporters

Conversation

@lokeshj1703
Copy link
Copy Markdown
Collaborator

Change Logs

The PR aims to add support for multiple metric reporters within a Metric Registry. Further it also adds labels to metrics.

Impact

NA

Risk level (write none, low medium or high below)

low

Documentation Update

Describe any necessary documentation update if there is any new feature, config, or user-facing change

  • The config description must be updated if new configs are added or the default value of the configs are changed
  • Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
    ticket number here and follow the instruction to make
    changes to the website.

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

reporterList.add(MetricsReporterFactory.createReporter(secondarySourceConfig, registry));
}
} catch (IOException e) {
LOG.error("Failed to add MetricsExporters", e);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's remove this redundant log? the exception below already includes it.

@nsivabalan
Copy link
Copy Markdown
Contributor

can you take a look at #7934 and ensure there are no overlaps or do you need any coordination.

@lokeshj1703
Copy link
Copy Markdown
Collaborator Author

PR 7934 adds changes for supporting metric instances for different tables. This PR adds support for multiple reporters within a metric instance.
There would definitely be conflicts between these two PRs but otherwise PRs are tackling different issues.

@nsivabalan
Copy link
Copy Markdown
Contributor

sg

@lokeshj1703 lokeshj1703 force-pushed the multipleReporters branch 2 times, most recently from 9dca0ad to a8cdea9 Compare March 1, 2023 06:24
@hudi-bot
Copy link
Copy Markdown
Collaborator

hudi-bot commented Mar 3, 2023

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@nsivabalan
Copy link
Copy Markdown
Contributor

CI is green
image

@codope codope merged commit 81e6e85 into apache:master Mar 3, 2023
fengjian428 pushed a commit to fengjian428/hudi that referenced this pull request Apr 5, 2023
…ls (apache#8041)

Add support for multiple metric reporters within a MetricRegistry. 
Further it also adds labels to metrics.
stayrascal pushed a commit to stayrascal/hudi that referenced this pull request Apr 20, 2023
…ls (apache#8041)

Add support for multiple metric reporters within a MetricRegistry. 
Further it also adds labels to metrics.
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.

4 participants