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 an option to force using hostname instead of FQDN in the resource detection processor #5064

Merged
merged 12 commits into from
Sep 21, 2021

Conversation

hprajapati-splunk
Copy link
Contributor

@hprajapati-splunk hprajapati-splunk commented Sep 2, 2021

Description:
This PR adds an option to force using hostname instead of FQDN in the resource detection processor.

Users can set hostname_sources to ["os"] to force the processor to use 'hostname'.

This is an opt-in functionality so there are no breaking changes to existing user configurations

Testing:
Existing unit tests have been updated and new unit tests have been added to cover new changes.

Documentation:
README has been updated to show how to use the new custom 'system' configurations

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 2, 2021

CLA Signed

The committers are authorized under a signed CLA.

@project-bot project-bot bot added this to In progress in Collector Sep 2, 2021
@mx-psi
Copy link
Member

mx-psi commented Sep 2, 2021

Also @hprajapati-splunk you need to sign the CLA so that we can accept your contribution, PTAL at the bot message above!

Copy link
Member

@mx-psi mx-psi 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 bearing with us on the review :) Behavior-wise this looks good to me but I left a couple of minor comments about the code. Also, note that CI will be broken until #5076 gets merged (sorry about that!)

Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Looks good, thanks again! I would like @dmitryax to have a look at it too

Collector automation moved this from In progress to Reviewer approved Sep 3, 2021
@bogdandrutu
Copy link
Member

@dmitryax PTAL also set the "ready-to-merge" label when you are done :)

processor/resourcedetectionprocessor/README.md Outdated Show resolved Hide resolved
processor/resourcedetectionprocessor/README.md Outdated Show resolved Hide resolved
processor/resourcedetectionprocessor/factory.go Outdated Show resolved Hide resolved
@dmitryax dmitryax added the ready to merge Code review completed; ready to merge by maintainers label Sep 15, 2021
Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
@bogdandrutu bogdandrutu merged commit a9a32ac into open-telemetry:main Sep 21, 2021
Collector automation moved this from Reviewer approved to Done Sep 21, 2021
hex1848 pushed a commit to hex1848/opentelemetry-collector-contrib that referenced this pull request Jun 2, 2022
This includes changes to support optional types:
- update sed script to update optional type to oneof
- bump proto to 0.15.0
- make genproto
- update sum to OptionalType
- InstrumentationLibraryMetrics -> ScopeMetrics
- InstrumentationLibrarySpans -> ScopeSpans
- InstrumentationLibraryLogs -> ScopeLogs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
No open projects
Collector
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants