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

Update Docs and Table to reflect appropriate information #223

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

vasireddy99
Copy link
Contributor

Description:
This PR updates the language parity table and update information in README.md

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@bryan-aguilar bryan-aguilar left a comment

Choose a reason for hiding this comment

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

Why is AWS X-Ray Trace ID injection into application logs NA for all languages except Java?

Can you link the docs for the .NET resource detectors for EKS/ECS?

@vasireddy99
Copy link
Contributor Author

Why is AWS X-Ray Trace ID injection into application logs NA for all languages except Java?

Can you link the docs for the .NET resource detectors for EKS/ECS?

Sure, here is info about the resource Detectors in donet

Trace ID injection is currently available in Java and is not implemented in other language SDK's.

Copy link
Member

@mhausenblas mhausenblas left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM overall. Can you pls address my inline comments and we can merge thereafter?

README.md Outdated Show resolved Hide resolved
|AWS X-Ray Trace ID injection into application logs |Done | NA | NA | NA | NA |
|Metadata/Annotations |Done (through ADOT Collector) |Done (through ADOT Collector) |Done (through ADOT Collector) |Done (through ADOT Collector) |Done (Through ADOT Collector) |
|AWS Lambda support |Done |Done | NA |Done (with auto-instrumentation) | NA |
|Auto-instrumentation |Done |Done |Needs implementation in OTel |Done |Needs implementation in OTel |
Copy link
Member

@mhausenblas mhausenblas Jan 20, 2023

Choose a reason for hiding this comment

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

Can we make sure that wherever we have "Needs implementation in OTel" there is a link to the upstream issue to aid tracking?

Copy link
Contributor Author

@vasireddy99 vasireddy99 Jan 21, 2023

Choose a reason for hiding this comment

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

For NodeJS, I see that aws-sdk and aws-lambda and not any specific issues to it.
Would like to double check if we are reflecting right status, if not I can modify the status for this DONE.

For .Net - We have listed a TODO in the docs, but couldn't find any tracking issue or dicsussions.

cc'ing @willarmiros to help with any info that we might be missing here about tracking issues.

@vasireddy99 vasireddy99 changed the title Update Docs and Table to reflect appropriate inforamtion Update Docs and Table to reflect appropriate information Jan 21, 2023
@willarmiros
Copy link
Contributor

Why is AWS X-Ray Trace ID injection into application logs NA for all languages except Java?

+1, same question. This should be a TODO in all languages except Java, not N/A.

Copy link
Contributor

@willarmiros willarmiros left a comment

Choose a reason for hiding this comment

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

Thanks for updating this, it's awesome to share w/ our customers! In the future, it would be even cooler if we could have links to the corresponding docs for each cell :)

language_parity_chart.md Outdated Show resolved Hide resolved
language_parity_chart.md Outdated Show resolved Hide resolved
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.

4 participants