Skip to content

Add resource detectors to cart service#663

Merged
cartersocha merged 6 commits into
open-telemetry:mainfrom
svrnm:add-resource-detectors-to-cart-service
Jan 7, 2023
Merged

Add resource detectors to cart service#663
cartersocha merged 6 commits into
open-telemetry:mainfrom
svrnm:add-resource-detectors-to-cart-service

Conversation

@svrnm

@svrnm svrnm commented Dec 28, 2022

Copy link
Copy Markdown
Member

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
Comment thread CHANGELOG.md

@julianocosta89 julianocosta89 left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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 :/

@julianocosta89 julianocosta89 left a comment

Copy link
Copy Markdown
Member

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

mviitane commented Jan 2, 2023

Copy link
Copy Markdown
Member

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

@svrnm

svrnm commented Jan 2, 2023

Copy link
Copy Markdown
Member Author

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
Copy Markdown
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
mohamed3637 added a commit to mohamed3637/opentelemetry-demo that referenced this pull request Oct 7, 2025
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