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

WithContainerID: Extracts incorrect container ID in ECS. #3633

Closed
mackjmr opened this issue Jan 31, 2023 · 4 comments
Closed

WithContainerID: Extracts incorrect container ID in ECS. #3633

mackjmr opened this issue Jan 31, 2023 · 4 comments
Labels
area:resources Part of OpenTelemetry resources bug Something isn't working

Comments

@mackjmr
Copy link
Member

mackjmr commented Jan 31, 2023

Description

WithContainerID (link) extracts the container ID from /proc/self/cgroup using the following regex. The container ID in ECS does not does not match the expected format of the regex.

  • Example working cgroup:
14:name=systemd:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
13:rdma:/
12:pids:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
11:hugetlb:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
10:net_prio:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
9:perf_event:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
8:net_cls:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
7:freezer:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
6:devices:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
5:memory:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
4:blkio:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
3:cpuacct:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
2:cpu:/docker/8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8
1:cpuset:/docker/c

Extracted container ID => 8c1e75bb325eb9c64571326952a6507132f1fc7eba7cd5b694422abe39fc7ed8

  • Example cgroup in ECS (docker container deployed with fargate):
11:freezer:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121
10:devices:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121
9:blkio:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121
8:perf_event:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121
7:pids:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121
6:memory:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121
5:cpuset:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121
4:cpu,cpuacct:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121
3:net_cls,net_prio:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121
2:hugetlb:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121
1:name=systemd:/ecs/121c14dc9fd841a5a995bdfb82dd4e32/121c14dc9fd841a5a995bdfb82dd4e32-2232344121	

Extracted container ID => 2232344121
Expected container ID => 121c14dc9fd841a5a995bdfb82dd4e32-2232344121

Environment

  • Operating system/Architecture: Linux/X86_64
  • Platform version: 1.4.0
  • Lauch type: FARGATE

Steps To Reproduce

Deploy a docker container with Fargate and use WithContainerID().

Expected behavior

The valid container ID to be added as a resource attribute in ECS.

One way to do this would be to adapt the regex if it is detected to be a Fargate environment.

Another way to do this is by leveraging ECS_CONTAINER_METADATA_URI (link). This endpoint returns a json containing the DockerId (and other metadata about the container itself). opentelemetry-java uses this method to detect resource attributes on ECS (PR).

@mackjmr mackjmr added the bug Something isn't working label Jan 31, 2023
@MrAlias
Copy link
Contributor

MrAlias commented Jan 31, 2023

cc @Aneurysm9

@MrAlias MrAlias added the area:resources Part of OpenTelemetry resources label Jan 31, 2023
@Aneurysm9
Copy link
Member

Aneurysm9 commented Jan 31, 2023

The ECS resource detector can correctly determine the continer ID. I don't know that WithContainerID() should be modified to special-case for ECS when a separate detector is already available.

@mackjmr
Copy link
Member Author

mackjmr commented Jan 31, 2023

Thanks for the response @Aneurysm9, the approach you mentioned makes sense to me. I think it would still be helpful to document this limitation in WithContainerID() and suggest using the ECS resource detector instead. Does that make sense? If so, happy to put up the PR to document this.

@Aneurysm9
Copy link
Member

I think it would still be helpful to document this limitation in WithContainerID() and suggest using the ECS resource detector instead. Does that make sense? If so, happy to put up the PR to document this.

That sounds good to me.

mackjmr added a commit to mackjmr/opentelemetry-go that referenced this issue Feb 3, 2023
WithContainerID is not able to extract the correct container id in an ECS environment. The ECS resource detector should be used instead (https://pkg.go.dev/go.opentelemetry.io/contrib/detectors/aws/ecs).
See open-telemetry#3633.
MrAlias added a commit that referenced this issue Feb 9, 2023
* WithContainerID: Document ECS limitation.

WithContainerID is not able to extract the correct container id in an ECS environment. The ECS resource detector should be used instead (https://pkg.go.dev/go.opentelemetry.io/contrib/detectors/aws/ecs).
See #3633.

* fix lint

* Update sdk/resource/config.go

---------

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@mackjmr mackjmr closed this as completed Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:resources Part of OpenTelemetry resources bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants