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 resource detectors to cart service #663

Merged

Conversation

svrnm
Copy link
Member

@svrnm svrnm commented Dec 28, 2022

Changes

Resource detection for cart service, right now it is only the container.id because it looks like no other resource detectors exist for .NET (yet)

Screenshot 2022-12-28 at 12 16 06

Merge Requirements

For new features contributions please make sure you have completed the following
essential items:

  • CHANGELOG.md updated to document new feature additions
  • Appropriate documentation updates in the docs folder

@svrnm svrnm requested a review from a team December 28, 2022 11:16
Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

Tried it out locally and nothing was added to the Resource Attributes. I suspect this is not working on my Ubuntu, because a similar issue happened when testing #662

@julianocosta89
Copy link
Member

Is it any specific version of docker required to this to work?
Ran docker system prune -a, re-built all the images, but still no success :/

Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

Tested in my Windows and it worked.
Also using docker

@mviitane
Copy link
Member

mviitane commented Jan 2, 2023

Doesn't work for me when using docker desktop on mac.

@svrnm
Copy link
Member Author

svrnm commented Jan 2, 2023

This is probably due to a Limitation because of cgroup v2, there is a solution in the making for dotnet:

open-telemetry/opentelemetry-dotnet-contrib#839

Java & NodeJS have that fixed already.

@cartersocha
Copy link
Contributor

Thanks @svrnm. Lgtm and the dependency PR seems to be making progress.

Merging.

@cartersocha cartersocha merged commit 35a2988 into open-telemetry:main Jan 7, 2023
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
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.

5 participants