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

Convert grpc-js plugin to instrumentation #1657

Closed
obecny opened this issue Nov 5, 2020 · 4 comments · Fixed by #1806
Closed

Convert grpc-js plugin to instrumentation #1657

obecny opened this issue Nov 5, 2020 · 4 comments · Fixed by #1806
Assignees

Comments

@obecny
Copy link
Member

obecny commented Nov 5, 2020

  • Change folder name from opentelemetry-plugin-* to opentelemetry-instrumentation-*
  • Change package name from @opentelemetry/plugin-* to @opentelemetry/instrumentation-*
  • Change implementation to use InstrumentationBase
@obecny obecny added feature-request up-for-grabs Good for taking. Extra help will be provided by maintainers labels Nov 5, 2020
@vmarchaud
Copy link
Member

I'm currently porting the grpc plugin and i was wondering if we could do the same as http/https and migrate both in one instrumentation ? This would remove the need of a package to share tests.

WDYT ? cc @obecny @dyladan

@obecny
Copy link
Member Author

obecny commented Dec 14, 2020

Can both plugins exists without each other in production ? if so I would not merge them.

@vmarchaud
Copy link
Member

They can, however in the near term grpc will be deprecated in favor of grpc-js. Also since we can instrument multiple modules at once i think it should handle both of them, it would reduce the code duplication between both packages and smoothly handle people upgrading from grpc to grpc-js

@obecny
Copy link
Member Author

obecny commented Dec 14, 2020

They can, however in the near term grpc will be deprecated in favor of grpc-js. Also since we can instrument multiple modules at once i think it should handle both of them, it would reduce the code duplication between both packages and smoothly handle people upgrading from grpc to grpc-js

It makes sense to merge it then .

@vmarchaud vmarchaud self-assigned this Dec 19, 2020
@vmarchaud vmarchaud removed the up-for-grabs Good for taking. Extra help will be provided by maintainers label Dec 19, 2020
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants