Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Add AggregatedMetrics to support multiple Metrics implementations#2917

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@Donnerbart
Copy link
Contributor

Partiallyfixes#2884
Alternative to#2912

See#2912 (comment)

CopilotAI review requested due to automatic review settingsAugust 27, 2025 16:15

This comment was marked as outdated.

@DonnerbartDonnerbartforce-pushed theimprovement/2884-add-aggregate-metrics branch fromf36afa7 to2528836CompareAugust 27, 2025 16:23

This comment was marked as outdated.

@DonnerbartDonnerbartforce-pushed theimprovement/2884-add-aggregate-metrics branch from2528836 toe80267dCompareAugust 27, 2025 16:27
Copy link

CopilotAI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces anAggregatedMetrics class that implements the composite pattern to aggregate multipleMetrics implementations, allowing simultaneous collection of metrics data by different monitoring systems.

Key changes:

  • NewAggregatedMetrics class that delegates calls to multiple metrics instances
  • Special handling fortimeControllerExecution() which only delegates to the first metrics instance
  • Comprehensive test suite with 100% coverage of the new functionality

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

FileDescription
AggregatedMetrics.javaCore implementation of the composite metrics aggregator with delegation logic
AggregatedMetricsTest.javaComplete test suite covering constructor validation, method delegation, and special cases
pom.xmlAdded mockito-core test dependency for unit testing

Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.

@Donnerbart
Copy link
ContributorAuthor

The new class should probably go into the same package as theMetrics interface. It has nothing to do with the concrete MicroMeter implementation. Will update the PR tomorrow.

* All method calls are delegated to each metrics instance in the list in the order they were
* provided to the constructor.
*
* <p><strong>Important:</strong> The {@link #timeControllerExecution(ControllerExecution)} method
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point. I forgot about that when I suggested this design! 😓

Copy link
Collaborator

@xstefankxstefank left a comment

Choose a reason for hiding this comment

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

LGTM

@csviri
Copy link
Collaborator

csviri commentedAug 28, 2025•
edited
Loading

Hi@Donnerbart loogks, great, would be better if you could targetnext branch, for the next minor version. Maybe also to mention this addon on docs (herehttps://javaoperatorsdk.io/docs/documentation/observability/ ). otherwise great, thank you

@DonnerbartDonnerbartforce-pushed theimprovement/2884-add-aggregate-metrics branch frome80267d to255ef8aCompareAugust 28, 2025 08:59
@DonnerbartDonnerbart changed the base branch frommain tonextAugust 28, 2025 09:00
@DonnerbartDonnerbartforce-pushed theimprovement/2884-add-aggregate-metrics branch 2 times, most recently from425634c to3e80997CompareAugust 28, 2025 09:01
…tionsSigned-off-by: David Sondermann <david.sondermann@hivemq.com>
@DonnerbartDonnerbartforce-pushed theimprovement/2884-add-aggregate-metrics branch from3e80997 toac8e46eCompareAugust 28, 2025 09:04
Signed-off-by: David Sondermann <david.sondermann@hivemq.com>
@Donnerbart
Copy link
ContributorAuthor

  • Moved the class into theio.javaoperatorsdk.operator.api.monitoring package.
  • Changed the target branch tonext.
  • Added and improved the documentation.
metacosm and csviri reacted with thumbs up emoji

@csviricsviri merged commita86fc2c intooperator-framework:nextAug 28, 2025
25 checks passed
@DonnerbartDonnerbart deleted the improvement/2884-add-aggregate-metrics branchAugust 28, 2025 11:24
csviri pushed a commit that referenced this pull requestSep 2, 2025
)* feature: add AggregatedMetrics to support multiple Metrics implementationsSigned-off-by: David Sondermann <david.sondermann@hivemq.com>
csviri pushed a commit that referenced this pull requestSep 24, 2025
)* feature: add AggregatedMetrics to support multiple Metrics implementationsSigned-off-by: David Sondermann <david.sondermann@hivemq.com>
csviri pushed a commit that referenced this pull requestOct 7, 2025
)* feature: add AggregatedMetrics to support multiple Metrics implementationsSigned-off-by: David Sondermann <david.sondermann@hivemq.com>
@csviricsviri mentioned this pull requestOct 13, 2025
csviri pushed a commit that referenced this pull requestOct 14, 2025
)* feature: add AggregatedMetrics to support multiple Metrics implementationsSigned-off-by: David Sondermann <david.sondermann@hivemq.com>
csviri pushed a commit that referenced this pull requestOct 18, 2025
)* feature: add AggregatedMetrics to support multiple Metrics implementationsSigned-off-by: David Sondermann <david.sondermann@hivemq.com>
csviri pushed a commit that referenced this pull requestOct 23, 2025
)* feature: add AggregatedMetrics to support multiple Metrics implementationsSigned-off-by: David Sondermann <david.sondermann@hivemq.com>
csviri pushed a commit that referenced this pull requestNov 20, 2025
)* feature: add AggregatedMetrics to support multiple Metrics implementationsSigned-off-by: David Sondermann <david.sondermann@hivemq.com>
csviri pushed a commit that referenced this pull requestNov 25, 2025
)* feature: add AggregatedMetrics to support multiple Metrics implementationsSigned-off-by: David Sondermann <david.sondermann@hivemq.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

Copilot code reviewCopilotCopilot left review comments

@metacosmmetacosmmetacosm approved these changes

@csviricsviricsviri approved these changes

@xstefankxstefankxstefank approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Custom bootstrap and CR lifecycle management

4 participants

@Donnerbart@csviri@metacosm@xstefank
[8]ページ先頭
©2009-2025 Movatter.jp