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 extra heuristic for hostname #838

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

SylvainJuge
Copy link
Member

@SylvainJuge SylvainJuge commented Nov 20, 2023

When using very minimal container images like the distroless, the hostname command line tool that our agents use to capture the host name is missing and can't be used.

When no value is captured for host name, the most obvious symptom is missing metrics in Kibana, the traces are not impacted.

While the HOST and HOSTNAME environment variables allow to provide a fallback, adding them to very large deployments can become an extra burden.

However, reading the content of the /etc/hostname is still possible and we should then use it to provide a value to the host name. In this case we don't know if a simple host name or an FQDN is provided.

In containers the actual value is usually the container ID or a random string thus it's not for "human consumption" but is still relevant for correlation.

In short, this just adds an extra fallback option to existing heuristics.


  • May the instrumentation collect sensitive information, such as secrets or PII (ex. in headers)?
    • n/a
  • Create PR as draft
  • Approval by at least one other agent
  • Mark as Ready for Review (automatically requests reviews from all agents and PM via CODEOWNERS)
    • Remove PM from reviewers if impact on product is negligible
    • Remove agents from reviewers if the change is not relevant for them
  • Approved by at least 2 agents + PM (if relevant)
  • Merge after 7 days passed without objections
    To auto-merge the PR, add /schedule YYYY-MM-DD to the PR description.
  • Create implementation issues through the meta issue template (this will automate issue creation for individual agents)
  • If this spec adds a new dynamic config option, add it to central config. n/a

Copy link
Member

@trentm trentm left a comment

Choose a reason for hiding this comment

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

LGTM.

(Note that the Node.js APM agent won't add this to its implementation, because it is already using the "equivalent" of hostname -f that doesn't depend on /bin/hostname existing. It currently uses Node.js's os.hostname(), which, IIRC, is using the gethostname syscall. (Earlier notes here: #805 (review))

@SylvainJuge SylvainJuge marked this pull request as ready for review November 22, 2023 08:30
@SylvainJuge SylvainJuge requested review from a team as code owners November 22, 2023 08:30
@Mpdreamz
Copy link
Member

Similar in .NET we don't call hostname directly.

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