search

jmxtrans / jmxtrans-agent

Java Agent based JMX metrics exporter.
MIT License
179 stars 110 forks source link

Refactored the tags parsing and message construction so it can be sub… #153

Closed endoplasmicR closed 3 years ago

endoplasmicR commented 3 years ago

…classed more easily. This is needed since main fields (e.g. statsType, tags) are private instead of protected, which means that the child class can not override the parsing easily.

cyrille-leclerc commented 3 years ago

Many thanks @endoplasmicR . As @billonahill suggested, could you please revert the formatting changes so that the diff is easier to read?

endoplasmicR commented 3 years ago

hey @cyrille-leclerc thanks for the comment and merging the change.

endoplasmicR commented 3 years ago

hi, @cyrille-leclerc can you help with merging this PR? also, can we get a new release if possible? Thank you

cyrille-leclerc commented 3 years ago

Can you please test https://github.com/jmxtrans/jmxtrans-agent/releases/tag/jmxtrans-agent-1.2.11-rc-2 and i'll cut a release. Thanks again for your contribution.

endoplasmicR commented 3 years ago

Can you please test https://github.com/jmxtrans/jmxtrans-agent/releases/tag/jmxtrans-agent-1.2.11-rc-2 and i'll cut a release. Thanks again for your contribution.

Will do, thanks for merging

endoplasmicR commented 3 years ago

hey, @cyrille-leclerc we are running into an interesting issue with:

java.lang.NoSuchMethodError: java.nio.ByteBuffer.flip()Ljava/nio/ByteBuffer;

Triggered from https://github.com/jmxtrans/jmxtrans-agent/blob/master/src/main/java/org/jmxtrans/agent/StatsDOutputWriter.java#L231

I think the root cause is exactly https://stackoverflow.com/questions/61267495/exception-in-thread-main-java-lang-nosuchmethoderror-java-nio-bytebuffer-flip

curious which JDK version was used to compile and release into maven?

cyrille-leclerc commented 3 years ago

@endoplasmicR I probably have used OpenJDK 15.

Precisely

java -version
openjdk version "15.0.2" 2021-01-19
OpenJDK Runtime Environment AdoptOpenJDK (build 15.0.2+7)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 15.0.2+7, mixed mode, sharing)

The explicit casting suggested in the stackoverflow article seems to be a good solution to me as it doesn't put constraints on the build process. What do you think?

endoplasmicR commented 3 years ago

@endoplasmicR I probably have used OpenJDK 15.

Precisely

java -version
openjdk version "15.0.2" 2021-01-19
OpenJDK Runtime Environment AdoptOpenJDK (build 15.0.2+7)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 15.0.2+7, mixed mode, sharing)

The explicit casting suggested in the stackoverflow article seems to be a good solution to me as it doesn't put constraints on the build process. What do you think?

Yes, I have submitted a PR https://github.com/jmxtrans/jmxtrans-agent/pull/156, can you update the RC version if possible?

Thank you.