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

HOSTNAME is blank when not present in environment #4405

Closed
nlindley opened this issue Jan 8, 2024 · 1 comment · Fixed by #4421
Closed

HOSTNAME is blank when not present in environment #4405

nlindley opened this issue Jan 8, 2024 · 1 comment · Fixed by #4421
Assignees
Labels
bug Something isn't working pkg:core priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization

Comments

@nlindley
Copy link

nlindley commented Jan 8, 2024

What happened?

Steps to Reproduce

Ensure HOSTNAME is not present on process.env

Expected Result

HOSTNAME falls back to os.hostname()

Actual Result

HOSTNAME is blank

Additional Details

It looks like the intention is to use os.hostname() as the default for HOSTNAME in getEnv(), but then it’s getting replaced by the default values from DEFAULT_ENVIRONMENT. I think the order of { HOSTNAME: os.hostname(), } and DEFAULT_ENVIRONMENT could be swapped.

return Object.assign(
{
HOSTNAME: os.hostname(),
},
DEFAULT_ENVIRONMENT,
processEnv
);

OpenTelemetry Setup Code

No response

package.json

No response

Relevant log output

No response

@nlindley nlindley added bug Something isn't working triage labels Jan 8, 2024
@pichlermarc pichlermarc added priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect pkg:resources and removed triage labels Jan 10, 2024
@pichlermarc pichlermarc self-assigned this Jan 16, 2024
@pichlermarc
Copy link
Member

Hi @nlindley thanks for reporting this. Looks like I mistakenly categorized this is a report for the EnvDetector resource detector, but there the results for that one seem correct. Also HostDetector returns the expected value. This is a report that concerns just getEnv(), correct?

If yes, then I think we'll do the following:

  • remove the broken os.hostname() fallback as it's not doing anything
  • keep the empty string as default
    • it's the parsing from the environment and if it is not set there, I think we should not apply any magic to it. While we don't use the HOSTNAME env var in our repo, other users might, and the might rely on that default being the empty string.

@pichlermarc pichlermarc added priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization and removed priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect labels Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:core priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants