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

Implement producer part of RocketMQ new client instrumentation #6884

Merged
merged 13 commits into from
Oct 28, 2022
Merged

Implement producer part of RocketMQ new client instrumentation #6884

merged 13 commits into from
Oct 28, 2022

Conversation

aaron-ai
Copy link
Member

Fix #6764 . This pull request is about the producer part.

@aaron-ai aaron-ai requested a review from a team October 14, 2022 12:56
@aaron-ai aaron-ai force-pushed the feature/pr_support_rocketmqclientjava branch from 182b728 to b5cd7d1 Compare October 17, 2022 02:54
@aaron-ai
Copy link
Member Author

Please take a look @trask @mateuszrzeszutek @laurit

@aaron-ai aaron-ai force-pushed the feature/pr_support_rocketmqclientjava branch from b5cd7d1 to 393d4a1 Compare October 19, 2022 05:56
laurit
laurit reviewed Oct 19, 2022
View reviewed changes
...umentation/rocketmq/rocketmq-client-java/rocketmq-client-java-5.0/javaagent/build.gradle.kts Outdated Show resolved Hide resolved
...umentation/rocketmq/rocketmq-client-java/rocketmq-client-java-5.0/javaagent/build.gradle.kts Outdated Show resolved Hide resolved
...umentation/rocketmq/rocketmq-client-java/rocketmq-client-java-5.0/javaagent/build.gradle.kts Outdated Show resolved Hide resolved
.../main/java/io/opentelemetry/javaagent/instrumentation/rocketmqclientjava/v5_0/MapSetter.java Outdated Show resolved Hide resolved
.../opentelemetry/javaagent/instrumentation/rocketmqclientjava/v5_0/ParentContextExtractor.java Outdated Show resolved Hide resolved
...telemetry/javaagent/instrumentation/rocketmqclientjava/v5_0/RocketMqInstrumenterFactory.java Outdated Show resolved Hide resolved
...telemetry/javaagent/instrumentation/rocketmqclientjava/v5_0/RocketMqInstrumenterFactory.java Outdated Show resolved Hide resolved
...telemetry/javaagent/instrumentation/rocketmqclientjava/v5_0/RocketMqInstrumenterFactory.java Outdated Show resolved Hide resolved
...ava/io/opentelemetry/instrumentation/rocketmqclientjava/v5_0/AbstractRocketMqClientTest.java Outdated Show resolved Hide resolved
...metry/javaagent/instrumentation/rocketmqclientjava/v5_0/RocketMqProducerInstrumentation.java Outdated Show resolved Hide resolved
@aaron-ai aaron-ai force-pushed the feature/pr_support_rocketmqclientjava branch 2 times, most recently from 4329278 to 2c182ed Compare October 20, 2022 09:40
@aaron-ai aaron-ai requested a review from laurit October 20, 2022 09:53
@aaron-ai aaron-ai force-pushed the feature/pr_support_rocketmqclientjava branch from 235bebd to 36e4537 Compare October 21, 2022 11:55
trask
trask reviewed Oct 23, 2022
View reviewed changes
settings.gradle.kts Outdated Show resolved Hide resolved
...ava/io/opentelemetry/instrumentation/rocketmqclientjava/v5_0/AbstractRocketMqClientTest.java Outdated Show resolved Hide resolved
...ava/io/opentelemetry/instrumentation/rocketmqclientjava/v5_0/AbstractRocketMqClientTest.java Outdated Show resolved Hide resolved
instrumentation/rocketmq/rocketmq-client-java/rocketmq-client-java-5.0/README.md Outdated Show resolved Hide resolved
.../main/java/io/opentelemetry/javaagent/instrumentation/rocketmqclientjava/v5_0/MapSetter.java Outdated Show resolved Hide resolved
.../opentelemetry/javaagent/instrumentation/rocketmqclientjava/v5_0/ParentContextExtractor.java Outdated Show resolved Hide resolved
...lemetry/javaagent/instrumentation/rocketmqclientjava/v5_0/RocketMqInstrumentationModule.java Outdated Show resolved Hide resolved
...lemetry/javaagent/instrumentation/rocketmqclientjava/v5_0/RocketMqInstrumentationModule.java Outdated Show resolved Hide resolved
...metry/javaagent/instrumentation/rocketmqclientjava/v5_0/RocketMqProducerInstrumentation.java Outdated Show resolved Hide resolved
@aaron-ai aaron-ai force-pushed the feature/pr_support_rocketmqclientjava branch 2 times, most recently from e394270 to 1211be2 Compare October 24, 2022 05:25
@aaron-ai aaron-ai mentioned this pull request Oct 25, 2022
Merged
@aaron-ai aaron-ai force-pushed the feature/pr_support_rocketmqclientjava branch from 2db0b42 to a150963 Compare October 25, 2022 06:12
@aaron-ai aaron-ai force-pushed the feature/pr_support_rocketmqclientjava branch from a150963 to 1e324bb Compare October 25, 2022 06:14
@aaron-ai aaron-ai force-pushed the feature/pr_support_rocketmqclientjava branch from 5b31404 to b75898b Compare October 26, 2022 06:30
@aaron-ai aaron-ai force-pushed the feature/pr_support_rocketmqclientjava branch from b75898b to cf3d2dc Compare October 26, 2022 06:52
@aaron-ai aaron-ai requested a review from laurit October 26, 2022 08:12
@aaron-ai aaron-ai requested review from trask and laurit and removed request for laurit and trask October 26, 2022 08:12
@aaron-ai
Copy link
Member Author

It seems that all issues mentioned above have been solved. any more feedback?

trask
trask approved these changes Oct 27, 2022
View reviewed changes
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

a couple of small suggestions👍

.../src/main/java/io/opentelemetry/javaagent/instrumentation/rocketmqclient/v5_0/MapSetter.java Outdated Show resolved Hide resolved
...telemetry/javaagent/instrumentation/rocketmqclient/v5_0/RocketMqProducerInstrumentation.java Outdated Show resolved Hide resolved
...telemetry/javaagent/instrumentation/rocketmqclient/v5_0/RocketMqProducerInstrumentation.java Outdated Show resolved Hide resolved
...telemetry/javaagent/instrumentation/rocketmqclient/v5_0/RocketMqProducerInstrumentation.java Outdated Show resolved Hide resolved
...aagent/instrumentation/rocketmqclient/v5_0/RocketMqPublishingMessageImplInstrumentation.java Outdated Show resolved Hide resolved
...aagent/instrumentation/rocketmqclient/v5_0/RocketMqPublishingMessageImplInstrumentation.java Outdated Show resolved Hide resolved
@aaron-ai
Copy link
Member Author

The markdown-link-check is failed, which will be fixed by #6986

@trask
Copy link
Member

trask commented Oct 27, 2022

@aaron-ai looks great 👍

will leave open for another day in case @laurit has a chance to take another look

@aaron-ai aaron-ai mentioned this pull request Oct 27, 2022
Closed
6 tasks
@trask trask merged commit 029ed3d into open-telemetry:main Oct 28, 2022
@aaron-ai aaron-ai deleted the feature/pr_support_rocketmqclientjava branch October 28, 2022 02:32
LironKS pushed a commit to helios/opentelemetry-java-instrumentation that referenced this pull request Oct 31, 2022
@aaron-ai @LironKS
Implement producer part of RocketMQ new client instrumentation (open-…
a08509f 
…telemetry#6884)

Fix open-telemetry#6764 . This pull request is about the producer part.
@aaron-ai aaron-ai mentioned this pull request Nov 1, 2022
Merged
@aaron-ai aaron-ai mentioned this pull request Nov 16, 2022
Closed
LironKS pushed a commit to helios/opentelemetry-java-instrumentation that referenced this pull request Dec 4, 2022
@aaron-ai @LironKS
Implement producer part of RocketMQ new client instrumentation (open-…
09e6cea 
…telemetry#6884)

Fix open-telemetry#6764 . This pull request is about the producer part.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trask trask trask approved these changes

@laurit laurit Awaiting requested review from laurit

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement instrumentation for the new client of Apache RocketMQ
3 participants
@aaron-ai @trask @laurit