Skip to content

Fix ECS resource detector populating container.name with the hostname - #4961

Open
rdbisme wants to merge 4 commits into
open-telemetry:mainfrom
rdbisme:fix/ecs-container-name
Open

Fix ECS resource detector populating container.name with the hostname#4961
rdbisme wants to merge 4 commits into
open-telemetry:mainfrom
rdbisme:fix/ecs-container-name

Conversation

@rdbisme

@rdbisme rdbisme commented Aug 15, 2026

Copy link
Copy Markdown

Description

The AwsEcsResourceDetector populates the container.name resource attribute with socket.gethostname(). On ECS tasks using the awsvpc network mode the container hostname is the IP-like private DNS name of the ENI (e.g. ip-10-0-2-100.us-west-2.compute.internal), so container.name ends up being an "IP name" instead of the actual container name.

When the metadata endpoint v4 is available (which the detector already queries), the container-level response contains the container name as defined in the task definition (field Name, e.g. curl). This PR uses that value for container.name when the v4 endpoint is present, and keeps the hostname as a fallback (v3-only path, or a response without a Name field).

No existing issue was filed for this; the fix follows the metadata response documented at https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-metadata-endpoint-v4.html#task-metadata-endpoint-v4-response

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • python -m pytest sdk-extension/opentelemetry-sdk-extension-aws/tests/resource/test_ecs.py — 4 passed.
    The v4 EC2/Fargate tests now mock socket.gethostname() to return the IP-like name while expecting container.name: "curl" from the metadata fixtures, proving the metadata value overrides the hostname. The v3 test keeps covering the hostname fallback.
  • ruff check on the changed source and test files — clean.

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@rdbisme
rdbisme requested a review from a team as a code owner August 15, 2026 09:29
@linux-foundation-easycla

linux-foundation-easycla Bot commented Aug 15, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 15, 2026

Copy link
Copy Markdown

Pull request dashboard status

Waiting on the author · refreshed 2026-08-25 16:06 UTC

Two things need attention:

  • Required checks are failing — investigate the failures.
  • 3 review items — respond to each (e.g. link a commit, explain why not, ask a follow-up):
    • Inline threads: 1
    • Top-level threads: 2, 3
Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
  • Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

@rdbisme

rdbisme commented Aug 16, 2026

Copy link
Copy Markdown
Author

@srprash @jj22ee

@HarperZ9

Copy link
Copy Markdown

Nice fix, and the v4 path looks right to me: metadata_container.get("Name", socket.gethostname()) merged over base_resource wins because Resource.merge keeps the non-empty value, so container.name becomes the ECS container name on v4.

One gap: the fix only covers the v4 path, but the title is about the hostname fallback in general. base_resource sets container.name = socket.gethostname() unconditionally, and the detector still admits v3-only environments (the entry guard returns empty only when both ECS_CONTAINER_METADATA_URI and ECS_CONTAINER_METADATA_URI_V4 are unset). On a v3-only container metadata_v4_endpoint is None, so if not metadata_v4_endpoint: return base_resource returns with container.name still set to the hostname, so the bug still happens there.

Two options: also read the v3 metadata endpoint (ECS_CONTAINER_METADATA_URI), which exposes Name as well, or explicitly document that v3-only falls back to the hostname. Either way a v3-only regression test would be worth adding, since the two added tests are both v4 (ec2 and fargate) and the v3-only path is uncovered.

Separate and softer, so treat this as a question: the v4 response carries both Name (the task-definition container name, "curl" in the fixture) and DockerName (the daemon-assigned runtime name, e.g. ecs-curltest-24-curl-...). semconv describes container.name as the name used by the container runtime, which reads closer to DockerName. Might be worth confirming which one the maintainers want. For what it is worth, the JS ECS detector uses os.hostname() today, so there is no settled cross-SDK precedent to match.

@xrmx xrmx moved this to Ready for review in Python PR digest Aug 25, 2026

@emdneto emdneto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok to keep the current behavior for the v3 metadata endpoint branch, because it's been deprecated for a while, although I agree one test proving a missing v4 Name falls back correctly would be nice. Also, the changelog needs to be updated explicitly about the change for the v4 metadata endpoint branch only.

I don't think DockerName is appropriate here; it won't be meaningful for troubleshooting purposes, so I'm ok with using the Name defined in the Task.

@github-project-automation github-project-automation Bot moved this from Ready for review to Reviewed PRs that need fixes in Python PR digest Aug 25, 2026
Comment on lines +63 to +67
container_name = metadata_container.get("Name", socket.gethostname())
base_resource = base_resource.merge(
Resource({ResourceAttributes.CONTAINER_NAME: container_name})
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
container_name = metadata_container.get("Name", socket.gethostname())
base_resource = base_resource.merge(
Resource({ResourceAttributes.CONTAINER_NAME: container_name})
)
container_name = metadata_container.get("Name")
if container_name:
base_resource = base_resource.merge(
Resource({ResourceAttributes.CONTAINER_NAME: container_name})
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Reviewed PRs that need fixes

Development

Successfully merging this pull request may close these issues.

4 participants