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

Add container ID attribute for Resource constants #1394

Closed
EdZou opened this issue Aug 6, 2020 · 3 comments
Closed

Add container ID attribute for Resource constants #1394

EdZou opened this issue Aug 6, 2020 · 3 comments
Assignees

Comments

@EdZou
Copy link
Contributor

EdZou commented Aug 6, 2020

Is your feature request related to a problem? Please describe.

I am currently implementing AWS ECS resource detector, following the Java implementation Java ECS Reource Detector
Since Java implementation parsed certain file and assign the parsed value to ResourceConstants.CONTAINER_ID, when I implement this part, I notice that there is no container ID attribute in JS resource constant.

Java Resource Constant: https://github.com/open-telemetry/opentelemetry-java/blob/f28396b812046bb69daf9899d777e140ff8de0e9/sdk/src/main/java/io/opentelemetry/sdk/resources/ResourceConstants.java#L54
JavaScript resource Constant: https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-resources/src/constants.ts#L34

Describe the solution you'd like

Should I add a container ID attribute for JS repo to make it consistent?

Describe alternatives you've considered

Is there other consideration about this issue like this is a JavaScript specific constant?
Or there is other JS attribute corresponding to container ID in Java version?

@dyladan
Copy link
Member

dyladan commented Aug 18, 2020

Should I add a container ID attribute for JS repo to make it consistent?

Yes. Would you like to be assigned?

@EdZou
Copy link
Contributor Author

EdZou commented Aug 18, 2020

Of course!

@mhennoch
Copy link
Contributor

Seems like this is fixed. @dyladan

pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
open-telemetry#1394)

* fix(amqplib): make all instrumentation methods private

Previously the `amqplib.d.ts` file would import `./utils` (resolved as
`utils.d.ts`) which imports `amqplib` directly. This can cause
typechecking issues for users who don't use `amqplib` but still import
the `amqplib` instrumentation through the node autoinstrumentation.

The `./utils` import was generated by typescript because some of the
protected methods have their argument and return types defined in
`utils.ts`. Now that these methods are private, typescript no longer
needs to generate the type information for those methods and omits the
`./utils` import.

Signed-off-by: Boris Bera <bbera@coveo.com>

* fix(amqplib): reduce the number of vendored types

Signed-off-by: Boris Bera <bbera@coveo.com>

---------

Signed-off-by: Boris Bera <bbera@coveo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants