Read lines with default encoding from os

[breedx-splk]

Repeat of #12419 which fixes #12418

@laurit had previously asked if a file could be provided to reproduce this. Unfortunately, it's problematic because it depends on the os encoding. The original problem was from z/OS.....which I have no reasonable way of reproducing with tests etc. It's odd that z/os wouldn't support or use UTF-8, but here we are.

This change should at least handle those edge cases.

[laurit]

To me using Charset.defaultCharset() doesn't seem like the best option. Since jdk18 the default charset is utf-8 unless user explicitly configures to use a different encoding, see https://openjdk.org/jeps/400. On older jdks users could use -Dfile.encoding=utf-8 even if the system encoding is something different.
Looking at https://github.com/moby/sys/blob/mountinfo/v0.7.2/mountinfo/mountinfo_linux.go#L23 I think they are also reading it as utf-8, could be wrong, don't really know any go. Without a sample that doesn't decode as with utf-8 it is hard to guess why it would fail. Could be that z/os does something weird. Also could be that the assumption of this file containing utf-8 is wrong and only works because everybody uses utf-8. Weren't paths just blobs from the linux kernel perspective? Instead of Charset.defaultCharset() I'd try StandardCharsets.ISO_8859_1. From what I understand we don't really care about the paths or whatever that could be in utf-8, container id is a hex string and the other symbols we use for parsing are all present in iso-8859-1.

[breedx-splk]

Since jdk18 the default charset is utf-8 unless user explicitly configures to use a different encoding, see https://openjdk.org/jeps/400.

Right, this PR is exactly a workaround for that specific case on older platforms (like z/os apparently) that don't use utf8 as a default. In other words, when we call the single-argument Files.lines() method, we get the jvm sensible default, which is utf8.

Instead of Charset.defaultCharset() I'd try StandardCharsets.ISO_8859_1.

🤯 You're blowing my mind with this recommendation. Just no. Maybe you have confidence in this, but I sure don't.

[laurit]

Since jdk18 the default charset is utf-8 unless user explicitly configures to use a different encoding, see https://openjdk.org/jeps/400.

Right, this PR is exactly a workaround for that specific case on older platforms (like z/os apparently) that don't use utf8 as a default. In other words, when we call the single-argument Files.lines() method, we get the jvm sensible default, which is utf8.

Let me rephrase, your proposed fix does not work on jdk18 and newer and might not work on older jdks if user configures utf-8 as the default encoding for jvm.

Instead of Charset.defaultCharset() I'd try StandardCharsets.ISO_8859_1.

🤯 You're blowing my mind with this recommendation. Just no. Maybe you have confidence in this, but I sure don't.

An alternative would be to read the file as bytes. If it decodes as utf-8 then all is fine, if it doesn't try another encoding. Considering that we only care about the 7-bit chars that are the same in utf-8 and iso-8859-1 imo just using iso-8859-1 to read the file would be simpler.

[breedx-splk]

your proposed fix does not work on jdk18 and newer and might not work on older jdks if user configures utf-8 as the default encoding for jvm

Can you help me to understand why you think that this won't work?

In the case of newer JVMs, as you pointed out, the default is already utf8, so Files.lines(path, Charset.defaultCharset()) should be effectively the same as Files.lines(path, Charset.forName("UTF-8") right? And this is effectively the same as Files.lines(path) which is what the code was before. This should still be fine, right?

This PR isn't trying to work-around the case where the user misconfigures the default encoding for the jvm (to mismatch the os, for example). It only works around the case where the jvm default is not utf-8 because the system default is not utf-8.

Considering that we only care about the 7-bit chars that are the same in utf-8 and iso-8859-1 [snip]

Not only am I not sure that we only care about the chars that are the same, I have not done the charset comparison to verify that the even ARE the same.

[laurit]

Can you help me to understand why you think that this won't work?

In the case of newer JVMs, as you pointed out, the default is already utf8, so Files.lines(path, Charset.defaultCharset()) should be effectively the same as Files.lines(path, Charset.forName("UTF-8") right? And this is effectively the same as Files.lines(path) which is what the code was before. This should still be fine, right?

This is all correct, the only thing you are forgetting is that reading the file as utf-8 is what is causing the issue in the first place. Using Files.lines(path, Charset.defaultCharset()) could fix the issue only if Charset.defaultCharset() does not return utf-8.

Considering that we only care about the 7-bit chars that are the same in utf-8 and iso-8859-1 [snip]

Not only am I not sure that we only care about the chars that are the same, I have not done the charset comparison to verify that the even ARE the same.

If you look at https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/CgroupV1ContainerIdExtractor.java and https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/CgroupV2ContainerIdExtractor.java you'll see that we only search for simple characters like /, : and return a hex string.
The 7bit character set is called ASCII. Both iso-8859-1 and utf-8 extend ascii, check the wikipedia links.

[breedx-splk]

Using Files.lines(path, Charset.defaultCharset()) could fix the issue only if Charset.defaultCharset() does not return utf-8.

Right, this is literally what is stated in the description of #12418 and what this PR addresses.

I've added a test, maybe that helps to add some clarity.