Fix empty host.id

[mwear]

Fixes #4312.

When reading a non-existent file, readFile was returning "", nil instead of "", err resulting in an empty host id. The original intent was to return an error in this case, but this will make the example in #4312 (pasted below) panic:

package main

import (
	"context"
	"fmt"

	"go.opentelemetry.io/otel/sdk/resource"
)

func main() {
	res, err := resource.New(context.Background(),
		resource.WithHostID(),
	)

	if err != nil {
		panic(err)
	}

	fmt.Println(res)
}

In a containerized environment, it's very likely that HostID detection will fail. That calls into question how we should handle this case. Should the hostIDDetector ignore errors and return an empty resource, or should it propagate the error (as per the original intent)? There is a similar detector in JS, but they work a little differently than the ones in the Go SDK. The HostDetector detects all host.* attributes. It will omit host.id if it's not found, but return the other found attributes in a new resource. I think the Go SDK equivalent would be to make the hostIDDetector a best-effort detector, and return an empty resource if it fails. Do any other detectors do this? Does anyone have thoughts or opinions? (cc: @mx-psi)

[codecov]

Codecov Report

Merging #4317 (a7369a3) into main (f6a658c) will increase coverage by 0.0%.
The diff coverage is 100.0%.

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #4317   +/-   ##
=====================================
  Coverage   83.4%   83.5%           
=====================================
  Files        184     184           
  Lines      14350   14350           
=====================================
+ Hits       11975   11983    +8     
+ Misses      2149    2141    -8     
  Partials     226     226           

Impacted Files	Coverage Δ
sdk/resource/host_id_readfile.go	100.0% <100.0%> (+100.0%)	⬆️

... and 2 files with indirect coverage changes

[MrAlias]

Should there be a test for this?

[mx-psi]

I think this is a net improvement, we can continue discussing what the behavior should actually be in a containerized setup separately.

To me silently failing sounds like "there is no valid host.id on this setup". This is not necessarily the case if we fail to read the files specified on the spec, since host.id could also be a cloud provider ID (e.g. AWS EC2 instance id). But I don't have context on what other detectors do to have a strong opinion here.

[pellared]

I think this is a net improvement, we can continue discussing what the behavior should actually be in a containerized setup separately.

To me silently failing sounds like "there is no valid host.id on this setup". This is not necessarily the case if we fail to read the files specified on the spec, since host.id could also be a cloud provider ID (e.g. AWS EC2 instance id). But I don't have context on what other detectors do to have a strong opinion here.

This PR fixes the bug. There should be a new issue (enhancement) that should be about providing a host.id value for the mentioned use cases.

[mwear]

Should there be a test for this?

readFile ends up getting mocked in all of the higher level tests. I can specifically add tests for the readFile function. Let me know if you'd like me to go that route.

[mwear]

Since readFile does not get exercised in normal tests due to mocking, I went ahead and added a couple of tests for it.

[MrAlias]

LGTM with the suggested CHANGELOG fix applied.