Opentelemetry-js: Hosting vendor supported exporters for OpenTelemetry

Created on 22 Jul 2020  ·  7Comments  ·  Source: open-telemetry/opentelemetry-js

AWS XRay Components

Description:

Hello everyone, I am an AWS intern working on adding Id generator, propagators and resource components for AWS X-Ray Exporter under JavaScript SDK repository.

Similar components exist in the Java implementation:

Id Generator: https://github.com/open-telemetry/opentelemetry-java/blob/a59748a904e0cb1e2e4f8df50dd346a82f26ff1e/sdk_extensions/aws_v1_support/src/main/java/io/opentelemetry/sdk/extensions/trace/aws/AwsXRayIdsGenerator.java#L36
Propagators: https://github.com/open-telemetry/opentelemetry-java/blob/a59748a904e0cb1e2e4f8df50dd346a82f26ff1e/extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/AwsXRayPropagator.java#L51
Resources: https://github.com/open-telemetry/opentelemetry-java/blob/a59748a904e0cb1e2e4f8df50dd346a82f26ff1e/sdk_extensions/aws_v1_support/src/main/java/io/opentelemetry/sdk/extensions/trace/aws/resource/AwsResource.java#L26

Questions:

  1. An existing implementation of id generator is here: https://github.com/open-telemetry/opentelemetry-js/blob/e9b2cf9aeb1daf5ffbab800681bfe1cafc636576/packages/opentelemetry-core/src/platform/node/id.ts. My questions are:

    1. The existing implementation only providing functions but not an IdGenerator interface in Go and Java implementation. So I also added an IdGenerator interface for JS repo and drafted a PR: https://github.com/open-telemetry/opentelemetry-js/pull/1331. My question is under which folder should I place the IdGenerator interface? “opentelemetry-api/src/platform/node” or like “opentelemetry-core/src/trace”

    2. Also, if I implement AWS IdGenerator (either an interface or exported functions as existing implementation), where should I put this part of code?

  2. If I want add an AWS X-Ray Propagator, where should I add it? Should I add this to the opentelemetry-js-contrib/propagator (https://github.com/open-telemetry/opentelemetry-js-contrib/tree/master/propagators) folder? Like the discussion here: https://github.com/open-telemetry/opentelemetry-specification/issues/637#issuecomment-644846847
  3. Similarly for resources, which repository are I supposed to add AWS Xray resources component? I did not see vendor repo for resource in both opentelemetry-js and opentelemetry-js-contrib repo.
question

Most helpful comment

Similarly for resources, which repository are I supposed to add AWS Xray resources component? I did not see vender repo for resource in both opentelemetry-js and opentelemetry-js-contrib repo.

I believe it should be hosted in seperate repo, see #890

It depends what is meant by "Resources". Is this simply resource detection, or some reporting of resources to x-ray? If it is detection, then the detector can live in the contrib repo. I have talked with other maintainers about this a few times and it seems like resource detection and possibly propagators may live in contrib, but that exporters should be hosted by the third party. Currently, the detectors live in our core resources package, but according to this, we should split them into their own packages.

The existing implementation only providing functions but not an IdGenerator interface in Go and Java implementation. So I also added an IdGenerator interface for JS repo and drafted a PR: #1331. My question is under which folder should I place the IdGenerator interface? “opentelemetry-api/src/platform/node” or like “opentelemetry-core/src/trace”

Not sure but i believe the interface (and the default implementation) should be in opentelemetry-core. The custom implementation for AWS should be done in a seperate repo and given as an option for the tracer.

I would add the interface to opentelemetry-core/src/trace/types.ts

If I want add an AWS X-Ray Propagator, where should I add it

I think we would need to wait for open-telemetry/opentelemetry-specification#637 (comment) to have an answer

I think it is probably ok for propagators to live in the contrib repo.

All 7 comments

Similarly for resources, which repository are I supposed to add AWS Xray resources component? I did not see vender repo for resource in both opentelemetry-js and opentelemetry-js-contrib repo.

I believe it should be hosted in seperate repo, see #890

The existing implementation only providing functions but not an IdGenerator interface in Go and Java implementation. So I also added an IdGenerator interface for JS repo and drafted a PR: #1331. My question is under which folder should I place the IdGenerator interface? “opentelemetry-api/src/platform/node” or like “opentelemetry-core/src/trace”

Not sure but i believe the interface (and the default implementation) should be in opentelemetry-core. The custom implementation for AWS should be done in a seperate repo and given as an option for the tracer.

If I want add an AWS X-Ray Propagator, where should I add it

I think we would need to wait for https://github.com/open-telemetry/opentelemetry-specification/issues/637#issuecomment-644846847 to have an answer

Similarly for resources, which repository are I supposed to add AWS Xray resources component? I did not see vender repo for resource in both opentelemetry-js and opentelemetry-js-contrib repo.

I believe it should be hosted in seperate repo, see #890

It depends what is meant by "Resources". Is this simply resource detection, or some reporting of resources to x-ray? If it is detection, then the detector can live in the contrib repo. I have talked with other maintainers about this a few times and it seems like resource detection and possibly propagators may live in contrib, but that exporters should be hosted by the third party. Currently, the detectors live in our core resources package, but according to this, we should split them into their own packages.

The existing implementation only providing functions but not an IdGenerator interface in Go and Java implementation. So I also added an IdGenerator interface for JS repo and drafted a PR: #1331. My question is under which folder should I place the IdGenerator interface? “opentelemetry-api/src/platform/node” or like “opentelemetry-core/src/trace”

Not sure but i believe the interface (and the default implementation) should be in opentelemetry-core. The custom implementation for AWS should be done in a seperate repo and given as an option for the tracer.

I would add the interface to opentelemetry-core/src/trace/types.ts

If I want add an AWS X-Ray Propagator, where should I add it

I think we would need to wait for open-telemetry/opentelemetry-specification#637 (comment) to have an answer

I think it is probably ok for propagators to live in the contrib repo.

Hello Daniel! @dyladan Really thank you for your time!
As we discussed on the SIG meeting this week, we may also open a folder for AWS Xray IdGenerator, my question is will JS group open a "js-contrib/idGenerator" folder for id generator components? So we could add a "js-contrib/idGenerator/AWSXRayIdGenerator"? Or elsewhere should we add a folder for it?
Also, just to make sure we are going to add "js-contrib/propagator/AWSXrayPropagator" folder for AWS Xray propagator.

Empty folders cannot be committed to github so the first id generator package will need to create it.
I think the names propagators and id-generators work fine.

Got it, thank you!

Final question for this issue.
I now know the exact requirement for AWS resources components, I am going to add AWS Beanstalk and Ecs resource detector, am I supposed to add code under opentelemetry-resources/src/platform/node? @dyladan

@EdZou unless it is time-sensitive, I would wait for the changes required by https://github.com/open-telemetry/oteps/pull/111 to land.

Specifically, resource detectors will be made to live in separate extension modules rather than living in our @opentelemetry/resources package.

Was this page helpful?
0 / 5 - 0 ratings