Skip to content

Netty metrics binders for micronaut-core-1.3.x#72

Merged
ilopmar merged 9 commits intomicronaut-projects:masterfrom
croudet:nettyMetricsBinders
Aug 24, 2020
Merged

Netty metrics binders for micronaut-core-1.3.x#72
ilopmar merged 9 commits intomicronaut-projects:masterfrom
croudet:nettyMetricsBinders

Conversation

@croudet
Copy link
Copy Markdown

@croudet croudet commented May 31, 2020

Add some netty metrics binders

- Expose default bytebuf allocator metrics: https://netty.io/4.1/api/io/netty/buffer/ByteBufAllocatorMetric.html, https://netty.io/4.1/api/io/netty/buffer/PooledByteBufAllocatorMetric.html
- Instrument EventLoopGroop Queues to expose their sizes, and tasks wait time in queues as well as tasks execution time.

@croudet croudet force-pushed the nettyMetricsBinders branch 4 times, most recently from a11957e to f5c0ddb Compare June 1, 2020 01:29
graemerocher
graemerocher previously requested changes Jun 1, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@graemerocher graemerocher left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Biggest problem I think is the cross package usage of io.micronaut.http.server.netty

Comment thread ...ore/src/main/java/io/micronaut/http/server/netty/InstrumentedEpollEventLoopGroupFactory.java Outdated
Comment thread ...re/src/main/java/io/micronaut/http/server/netty/InstrumentedKQueueEventLoopGroupFactory.java Outdated
Comment thread ...-core/src/main/java/io/micronaut/http/server/netty/InstrumentedNioEventLoopGroupFactory.java Outdated
Comment thread ...a/io/micronaut/configuration/metrics/binder/netty/InstrumentedEventLoopTaskQueueFactory.java Outdated
Comment thread ...meter-core/src/main/java/io/micronaut/configuration/metrics/binder/netty/MonitoredQueue.java Outdated
Comment thread ...meter-core/src/main/java/io/micronaut/configuration/metrics/binder/netty/MonitoredQueue.java
@croudet croudet force-pushed the nettyMetricsBinders branch 4 times, most recently from a2a39b9 to 9ce0b93 Compare June 1, 2020 14:29
croudet38 added 2 commits June 2, 2020 07:01
@croudet38
Merge remote-tracking branch 'upstream/master'
3a25d62
@croudet38
Add some netty metrics binders
4550ac2 
- Expose default bytebuf allocator metrics: https://netty.io/4.1/api/io/netty/buffer/ByteBufAllocatorMetric.html, https://netty.io/4.1/api/io/netty/buffer/PooledByteBufAllocatorMetric.html
- Instrument EventLoopGroop Queues to expose their sizes, and tasks wait time in queues as well as tasks execution time.
@croudet croudet mentioned this pull request Jun 2, 2020
Merged
croudet pushed a commit to croudet/micronaut-core that referenced this pull request Jun 2, 2020
@croudet38
Turn EventLoopGroupFactories public
89d3945 
required for micronaut-projects/micronaut-micrometer#72
@croudet croudet mentioned this pull request Jun 2, 2020
Merged
graemerocher pushed a commit to micronaut-projects/micronaut-core that referenced this pull request Jun 2, 2020
@croudet38
Turn EventLoopGroupFactories public (#3393)
155c0bf 
required for micronaut-projects/micronaut-micrometer#72
@croudet croudet force-pushed the nettyMetricsBinders branch from 9ce0b93 to f958f42 Compare June 2, 2020 13:45
@croudet38
clean-up code.
70995a1 
- Turn classes final and add @internal
- Add comments
- move methods

To be merged when micronaut=core-1.3.6 will be released.
Currently points to 1.3.6-SNAPSHOT.
@croudet croudet force-pushed the nettyMetricsBinders branch from f958f42 to 70995a1 Compare June 2, 2020 13:51
@croudet croudet changed the title Netty metrics binders Netty metrics binders for micronaut-core-1.3.x Jun 2, 2020
@graemerocher
Copy link
Copy Markdown
Contributor

graemerocher commented Jun 3, 2020

This PR would need to be against the 1.2.x branch for Micronaut 1.3.x

@croudet croudet changed the base branch from master to 1.2.x June 3, 2020 12:56
@croudet croudet closed this Jun 3, 2020
@croudet
Copy link
Copy Markdown
Author

croudet commented Jun 6, 2020

Tag v1.3.1 is based on master. I think a branch 1.3.x should be created for micronaut 1.3.x

@croudet croudet reopened this Jun 6, 2020
@croudet croudet changed the base branch from 1.2.x to master June 6, 2020 14:48
@alvarosanchez
Copy link
Copy Markdown
Member

alvarosanchez commented Jul 24, 2020

@graemerocher what's the status of this?

@graemerocher
Copy link
Copy Markdown
Contributor

graemerocher commented Jul 24, 2020

I think it can be closed, since it targets an older version. @croudet do you need another 1.3.x version?

@alvarosanchez
Copy link
Copy Markdown
Member

alvarosanchez commented Jul 24, 2020

#93 already contains Netty metrics binders no?

@graemerocher
Copy link
Copy Markdown
Contributor

graemerocher commented Jul 24, 2020

Yes

@croudet
Copy link
Copy Markdown
Author

croudet commented Jul 24, 2020

Some classes have been moved to another package in 2.0.
This is why there is 2 versions for this: one for micronaut-core 1.3.x and one that targets micronaut-core 2.x

@croudet
Copy link
Copy Markdown
Author

croudet commented Jul 24, 2020

My suggestion was to create a new branch here to seperate implementation for 1.3.x and 2.x

croudet38 added 2 commits July 26, 2020 18:18
@croudet38
Upgrade libs
5837db0
@croudet38
Remove Tiny subpages metrics as
ff0a923 
it will be deprecated in netty 4.1.52 and always
return 0.
netty/netty#10267
@alvarosanchez
Copy link
Copy Markdown
Member

alvarosanchez commented Jul 27, 2020

I'm not opposed to release a new version for Micronaut 1.3. It should be Micronaut Micrometer 2.1.

But then for #93, the project version should be 3.x.

If we agree, I'll create the release branches.

@alvarosanchez alvarosanchez self-assigned this Jul 27, 2020
@ilopmar
Copy link
Copy Markdown
Contributor

ilopmar commented Aug 24, 2020

@croudet what's the status of this? I'm working on upgrading the project to Micronaut 2. I see PR #93 for the upgrade which also contains the changes in this PR, right?
Can we merge this, release a new version and then do the upgrade merging #93?

ilopmar
ilopmar reviewed Aug 24, 2020
View reviewed changes
Comment thread gradle.properties Outdated
micronautVersion=1.3.7
micronautTestVersion=1.2.2
groovyVersion=2.5.11
nettyVersion=4.1.51.Final
Copy link
Copy Markdown
Contributor

@ilopmar ilopmar Aug 24, 2020

Choose a reason for hiding this comment

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

This version contains some regressions so we can't use it. We should stick at this moment with 4.1.48.Final as in micronaut-core

@ilopmar ilopmar self-assigned this Aug 24, 2020
@ilopmar ilopmar mentioned this pull request Aug 24, 2020
Merged
@croudet
Copy link
Copy Markdown
Author

croudet commented Aug 24, 2020

@croudet what's the status of this? I'm working on upgrading the project to Micronaut 2. I see PR #93 for the upgrade which also contains the changes in this PR, right?
Can we merge this, release a new version and then do the upgrade merging #93?

Yes, we can do that.

@alvarosanchez alvarosanchez removed their assignment Aug 24, 2020
@ilopmar ilopmar merged commit 63c0520 into micronaut-projects:master Aug 24, 2020
@ilopmar ilopmar added the type: enhancement New feature or request label Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@graemerocher graemerocher graemerocher left review comments

+1 more reviewer

@ilopmar ilopmar ilopmar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ilopmar ilopmar

Labels

type: enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@croudet @graemerocher @alvarosanchez @ilopmar @croudet38