Skip to content

NH-103815: update the default resource attributes#185

Merged
xuan-cao-swi merged 8 commits into
pure-otel-rubyfrom
NH-103815
May 13, 2025
Merged

NH-103815: update the default resource attributes#185
xuan-cao-swi merged 8 commits into
pure-otel-rubyfrom
NH-103815

Conversation

@xuan-cao-swi
Copy link
Copy Markdown
Contributor

@xuan-cao-swi xuan-cao-swi commented May 1, 2025

Description

Test (if applicable)

Azure platform; EC2; ContianerID

@xuan-cao-swi xuan-cao-swi requested a review from a team as a code owner May 1, 2025 15:55
@xuan-cao-swi xuan-cao-swi changed the base branch from main to pure-otel-ruby May 1, 2025 15:56
Comment on lines +23 to +24
RESOURCE_ATTRIBUTE = 'RESOURCE_ATTRIBUTE'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was this meant to go from plural to singular ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was just creating some name for in-code configuration, but I can change to plural (e.g. RESOURCE_ATTRIBUTES = 'RESOURCE_ATTRIBUTES')

require 'socket'
require 'securerandom'
require 'opentelemetry/resource/detector/azure'
require 'opentelemetry/resource/detector/aws/ec2' if RUBY_VERSION >= '3.1.0'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@xuan-cao-swi does the latest release https://github.com/open-telemetry/opentelemetry-ruby-contrib/releases/tag/opentelemetry-resource-detector-aws%2Fv0.3.0 mean we can remove this special casing for ruby 3.1.0 here and in the test_gems.gemfile file? If not, mind putting in a comment on why this is needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This gem require ruby version >= 3.1.0 (include its v0.1.0), so anyone who wants to use the aws resource detector has to use ruby >= 3.1.0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added comments.

Copy link
Copy Markdown
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @xuan-cao-swi!

@xuan-cao-swi xuan-cao-swi merged commit 7f5d6b8 into pure-otel-ruby May 13, 2025
6 of 8 checks passed
@xuan-cao-swi xuan-cao-swi deleted the NH-103815 branch June 9, 2025 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants