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.

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

Add TimedScheduler to reactor-core-micrometer module #3109

Merged
merged 21 commits into from Aug 4, 2022
Merged

Add TimedScheduler to reactor-core-micrometer module #3109

merged 21 commits into from Aug 4, 2022

Conversation

simonbasle
Copy link
Member

@simonbasle simonbasle commented Jul 7, 2022 •
edited

This is ultimately intended as a replacement for both Schedulers.enableMetrics() and Micrometer.enableSchedulersMetricsDecorator.

⚠️ warn/api-change compared to M3/M4

The scheduler-related methods in reactor-core-micrometer Micrometer class have been removed:

  • Micrometer#enableSchedulersMetricsDecorator()
  • Micrometer#disableSchedulersMetricsDecorator()

Either adapt code to use TimedScheduler or use reactor-core previous generation's (also deprecated) Schedulers#enableMetrics()

@simonbasle simonbasle added type/enhancement A general enhancement warn/deprecation This issue/PR introduces deprecations area/observability labels Jul 7, 2022
@simonbasle simonbasle added this to the 3.5.0-M4 milestone Jul 7, 2022
@simonbasle simonbasle self-assigned this Jul 7, 2022
@simonbasle simonbasle modified the milestones: 3.5.0-M4, 3.5.0-M5 Jul 7, 2022
@simonbasle simonbasle added the status/on-hold This was set aside or cannot be worked on yet label Jul 7, 2022
@simonbasle
Copy link
Member Author

This is probably a bit too late for a polished version to land in M4. Given how close this release is going to be to Spring milestone, we should probably instead defer to M5 (at which point we'd have more leeway to simply remove instead of deprecate, I think).

@simonbasle simonbasle mentioned this pull request Jul 18, 2022
Closed
@simonbasle simonbasle added warn/api-change Breaking change with compilation errors and removed warn/deprecation This issue/PR introduces deprecations labels Jul 25, 2022
@simonbasle simonbasle changed the title Add TimedScheduler to reactor-core-micrometer module [BREAKING from M4] Add TimedScheduler to reactor-core-micrometer module Jul 25, 2022
@simonbasle simonbasle marked this pull request as ready for review July 28, 2022 15:02
@simonbasle simonbasle requested a review from a team as a code owner July 28, 2022 15:02
chemicL
chemicL requested changes Aug 4, 2022
View reviewed changes
reactor-core-micrometer/src/main/java/reactor/core/observability/micrometer/Micrometer.java Outdated Show resolved Hide resolved
reactor-core-micrometer/src/main/java/reactor/core/observability/micrometer/Micrometer.java Outdated Show resolved Hide resolved
reactor-core-micrometer/src/main/java/reactor/core/observability/micrometer/Micrometer.java Outdated Show resolved Hide resolved
reactor-core-micrometer/src/main/java/reactor/core/observability/micrometer/Micrometer.java Outdated Show resolved Hide resolved
reactor-core-micrometer/src/main/java/reactor/core/observability/micrometer/TimedScheduler.java Outdated Show resolved Hide resolved
reactor-core-micrometer/src/main/java/reactor/core/observability/micrometer/TimedScheduler.java Show resolved Hide resolved
reactor-core-micrometer/src/main/java/reactor/core/observability/micrometer/TimedScheduler.java Outdated Show resolved Hide resolved
reactor-core-micrometer/src/main/java/reactor/core/observability/micrometer/TimedScheduler.java Outdated Show resolved Hide resolved
reactor-core-micrometer/src/main/java/reactor/core/observability/micrometer/TimedScheduler.java Outdated Show resolved Hide resolved
reactor-core-micrometer/src/main/java/reactor/core/observability/micrometer/TimedScheduler.java Outdated Show resolved Hide resolved
@simonbasle
fix review comments, notably 'periodic' wording
dc35b4d
@simonbasle
move tests from confusing case TimedSchedulerTest to SingleSchedulerT…
8859f80 
…est in core

an old test in core is named `TimedSchedulerTest`. This is confusing now
that there is an actual `TimedScheduler`. Since this test class uses the
SingleScheduler its two tests have been moved to `SingleSchedulerTest`
(and slightly amended to use `afterTest` extension).
chemicL
chemicL reviewed Aug 4, 2022
View reviewed changes
...-core-micrometer/src/test/java/reactor/core/observability/micrometer/TimedSchedulerTest.java Outdated Show resolved Hide resolved
...-core-micrometer/src/test/java/reactor/core/observability/micrometer/TimedSchedulerTest.java Outdated Show resolved Hide resolved
...-core-micrometer/src/test/java/reactor/core/observability/micrometer/TimedSchedulerTest.java Outdated Show resolved Hide resolved
...-core-micrometer/src/test/java/reactor/core/observability/micrometer/TimedSchedulerTest.java Outdated Show resolved Hide resolved
...-core-micrometer/src/test/java/reactor/core/observability/micrometer/TimedSchedulerTest.java Outdated Show resolved Hide resolved
...-core-micrometer/src/test/java/reactor/core/observability/micrometer/TimedSchedulerTest.java Outdated Show resolved Hide resolved
@simonbasle simonbasle changed the title [BREAKING from M4] Add TimedScheduler to reactor-core-micrometer module Add TimedScheduler to reactor-core-micrometer module Aug 4, 2022
@simonbasle simonbasle merged commit 06729ec into main Aug 4, 2022
@simonbasle simonbasle deleted the micrometerInstrumentedScheduler branch August 4, 2022 13:31
@simonbasle
Copy link
Member Author

relates to #1201

@simonbasle simonbasle mentioned this pull request Aug 8, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chemicL chemicL chemicL approved these changes

@jonatan-ivanov jonatan-ivanov jonatan-ivanov approved these changes

@OlegDokuka OlegDokuka OlegDokuka approved these changes

Assignees

@simonbasle simonbasle

Labels
area/observability status/on-hold This was set aside or cannot be worked on yet type/enhancement A general enhancement warn/api-change Breaking change with compilation errors
Projects
None yet
Milestone
3.5.0-M5
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@simonbasle @chemicL @jonatan-ivanov @OlegDokuka