Apm: [agents] Read docker container id from filesystem

[roncohen]

Agents should try to read the current container ID from the local filesystem. Elasticsearch already does this.

There's previously been discussion around issues that could arise because the agent could be blocked while fetching this information. I'm not too concerned about blocking the agent startup for a little bit while we check if the file exists and read/parse it. It should only take a few milliseconds. If we start supporting AWS Lambda and similar we could skip the check outright based on an environment variable that is automatically set by the provider.

There are some complications involved in parsing the file so it would be nice if one of the agents buildt a reference implementation that others can take model their own implementation on.

Pleas add the field to metadata as system.container.id.

The field will end up in Elasticsearch as container.id: https://github.com/elastic/ecs/blob/master/schemas/container.yml#L19-L23

@elastic/apm-agent-devs Please link your issue or PR and then mark the checkbox when the implementation is merged:

• [x] - Java - https://github.com/elastic/apm-agent-java/issues/325
• [x] - Python https://github.com/elastic/apm-agent-python/issues/346
• [x] - Ruby - https://github.com/elastic/apm-agent-ruby/issues/253
• [x] - Go - https://github.com/elastic/apm-agent-go/issues/328
• [x] - dotNet - https://github.com/elastic/apm-agent-dotnet/issues/238
• [x] - Node.js - https://github.com/elastic/apm-agent-nodejs/issues/693
• [x] - APM Server - https://github.com/elastic/apm-server/issues/1621

[axw]

Please only add the field to transactions and metrics.

Shouldn't this be going in metadata, and not in specific events?

[axw]

Also

The field should be container.id: https://github.com/elastic/ecs/blob/master/schemas/container.yml#L19-L23

That is relevant to the server. Where should we put it in the events we send to the server?

In metadata we currently have system, service, process, and user. We could add this as either container.id or system.container.id, seeing as in the case of containerization system relates to the container. Preferences?

[watson]

I vote for system.container.id, but inventing a new root property called container might also work 👍

[axw]

system.container.id is my preference also. My PR is here, in case anyone wants to copy: https://github.com/elastic/apm-agent-go/pull/330. I'll hold off on merging until we come to a decision on the schema change.

[roncohen]

you're right. I'll update the description. Thanks!

[watson]

As this is a blocking operation on boot, should we have a config option to disable it?

[axw]

As this is a blocking operation on boot, should we have a config option to disable it?

What are the conditions under which reading that path will block? Can you please point to some literature?

[beniwohli]

Does anybody have an exhaustive list of possible formatting that can occur in /proc/self/cgroup? Googling a bit, it doesn't seem to be a super stable file format.

[watson]

@axw It's in the issue description:

There's previously been discussion around issues that could arise because the agent could be blocked while fetching this information. I'm not too concerned about blocking the agent startup for a little bit while we check if the file exists and read/parse it

[beniwohli]

@watson /proc is a virtual file system and doesn't involve any disk I/O. So while reading a file from it is technically still a blocking operation, I don't think we have to expect it to be blocking startup.

[watson]

When I say blocking, I mean blocking in the technical sense. In Node.js _all_ I/O operations are non-blocking by default and the issue descriptions calls for switching that off for this task. This is probably fine, but every time we do this we have to be careful. By introducing a flag to switch this off we essentially add an escape hatch, so users who experience problems with it can disable it.

I'm not an expert in this file. Will it always be virtual, might it also exist on cloud functions, but behave differently? I don't know the answers to these questions, but while I don't except any problems, I just wanted to be cautious.

[axw]

@axw It's in the issue description:

There's previously been discussion around issues that could arise because the agent could be blocked while fetching this information. I'm not too concerned about blocking the agent startup for a little bit while we check if the file exists and read/parse it

@watson I saw that, but I don't remember any previous discussion. There's a statement there, but no links to any explanation of when it occurs. The latter is what I was after, since I was surprised to hear that this might block.

I'm not an expert in this file. Will it always be virtual, might it also exist on cloud functions, but behave differently? I don't know the answers to these questions, but while I don't except any problems, I just wanted to be cautious.

/proc/self/cgroup is part of procfs. There's nothing stopping something else being mounted at the same location, but I think it would be overly paranoid to defend against that.

[axw]

Does anybody have an exhaustive list of possible formatting that can occur in /proc/self/cgroup? Googling a bit, it doesn't seem to be a super stable file format.

@beniwohli man cgroups (7) states that the format is "hierarchy-ID:controller-list:cgroup-path". I presume you're referring to the cgroup-path field.

I couldn't locate any definitive list, but I did find references to two path formats:

• /system.slice/docker-<CID>.scope
• /docker/<CID>

where <CID> is the full container ID.

The approach I'm taking is:

1. take the basename of the cgroup-path
2. trim the "docker-" prefix and ".scope" suffix if found
3. check that the result is a 64-bit string of hex characters, to guard against future changes

My code is here: https://github.com/elastic/apm-agent-go/blob/b8507576e8559263cb3778ade974ed58638f76b6/internal/apmhostutil/docker_linux.go#L44-L45

[watson]

@axw I wouldn't be surprised if "the previously discussion" was on Slack and involved me raising the same concern as I'm raising here 😉

There's a few comments in this issue about what blocking means, so thought I'd just clarify: In our use-case it means that the agent "pauses" the entire app (blocks the main thread) while reading and parsing the file. Once the file is read and parsed the program is allowed to continue.

[watson]

But don't get me wrong. I'm ok with this being blocking. I just wanted to make sure that we had considered if it would be a good idea to add an escape hatch. If y'all think it's not important that's fine by me 😃

[Qard]

While /proc _is_ a virtual file system, and is therefore reading from memory, not disk, the open() and read() kernel methods to interact with it _are_ still subject to operation queuing and can therefore be significantly delayed in an I/O saturated environment.

[axw]

While /proc is a virtual file system, and is therefore reading from memory, not disk, the open() and read() kernel methods to interact with it are still subject to operation queuing and can therefore be significantly delayed in an I/O saturated environment.

The I/O scheduler operates only at the block level doesn't it? In this case, the open and read syscalls will use the "seq_file" API directly.

[eyalkoren]

I am assuming system.container.id is optional, but couldn't find it explicitly defined as such. Is it indeed?

[Qard]

@axw As far as I understand it: if you explicitly use the seq_file API, yes. Just using open(), it has to actually hit the disk before it can figure out what type it is to special-case the seq_file handling in read().

[axw]

@axw As far as I understand it: if you explicitly use the seq_file API, yes. Just using open(), it has to actually hit the disk before it can figure out what type it is to special-case the seq_file handling in read().

Disk access isn't required for all VFS lookups. In the case of something like /proc, you have /proc in the (in-core) mount table; the table entry contains a reference to the filesystem-specific operations struct, which is used to traverse to the desired path. There's also a shortcut via a cache. More in https://www.kernel.org/doc/Documentation/filesystems/vfs.txt. Happy to talk about this more, but we should probably take it somewhere else :)

[Qard]

Ah, didn't know the core kept mount data for /proc in-memory. I just know you can create arbitrary virtual filesystems anywhere, so the system isn't always aware of their existence until it gets that data from the disk. 🤷‍♂️

[alvarolobato]

This is finished.