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.

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

Rename grizzly-client instrumentation #1709

Merged
merged 3 commits into from
Nov 21, 2020
Merged

Rename grizzly-client instrumentation #1709

merged 3 commits into from
Nov 21, 2020

Conversation

trask
Copy link
Member

@trask trask commented Nov 20, 2020

Closes #1658

@iNikem
Copy link
Contributor

iNikem commented Nov 20, 2020

I think there is some confusion here.

https://github.com/AsyncHttpClient/async-http-client uses artifactId async-http-client and package name org.asynchttpclient
https://github.com/eclipse-ee4j/grizzly-ahc uses artifactId grizzly-http-client and package name com.ning.http.client.

The instrumentation under review monitors the second one, not the first one.

Copy link
Contributor

@iNikem iNikem left a comment

Choose a reason for hiding this comment

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

See my comment above

@trask
Copy link
Member Author

trask commented Nov 20, 2020

oh ya, com.ning.http.client was the package name prior to version 2.0, I've updated the README to note that we only support 1.9+ (not including 2.x yet)

@iNikem
Copy link
Contributor

iNikem commented Nov 20, 2020

Ok, I now understand better what is happening. But I am still not convinced this change is a good one. This instrumentation still was written for https://github.com/eclipse-ee4j/grizzly-ahc, judging from package and artifact names. Why do we want to instead say that this is about old version of https://github.com/AsyncHttpClient/async-http-client? Then we should change gradle build file and test with those artifacts, whose artifactId=async-http-client-project

@trask
Copy link
Member Author

trask commented Nov 21, 2020

Then we should change gradle build file and test with those artifacts

👍 done

@trask trask merged commit 081e142 into open-telemetry:master Nov 21, 2020
@trask trask deleted the rename-grizzly-client-module branch November 21, 2020 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename grizzly-client instrumentation to async-http-client
3 participants