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

Add semantic conventions for cloud provider-specific resource attributes #1099

Merged
merged 9 commits into from
Nov 25, 2020
Prev Previous commit
Next Next commit
switched cluster to cluster.arn
William Armiros committed Oct 28, 2020

Verified

This commit was signed with the committer’s verified signature.
snyk-bot Snyk bot
commit c66f5b13a9c92ea475a20a5dbcfc24833e2c3891
8 changes: 4 additions & 4 deletions semantic_conventions/resource/cloud_provider/aws/ecs.yaml
Original file line number Diff line number Diff line change
@@ -7,13 +7,13 @@ groups:
- id: container.arn
type: string
brief: >
The ARN of an [ECS container instance](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ECS_instances.html).
The Amazon Resource Name (ARN) of an [ECS container instance](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ECS_instances.html).
examples: ['arn:aws:ecs:us-west-1:123456789123:container/32624152-9086-4f0e-acae-1a75b14fe4d9']
- id: cluster
- id: cluster.arn
type: string
brief: >
The name of an [ECS cluster](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/clusters.html).
examples: ['opentelemetry-cluster']
The ARN of an [ECS cluster](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/clusters.html).
examples: ['arn:aws:ecs:us-west-2:123456789123:cluster/my-cluster']
- id: launchtype
type:
allow_custom_values: false
Original file line number Diff line number Diff line change
@@ -7,8 +7,8 @@
<!-- semconv aws.ecs -->
| Attribute | Type | Description | Example | Required |
|---|---|---|---|---|
| `aws.ecs.container.arn` | string | The ARN of an [ECS container instance](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ECS_instances.html). | `arn:aws:ecs:us-west-1:123456789123:container/32624152-9086-4f0e-acae-1a75b14fe4d9` | No |
| `aws.ecs.cluster` | string | The name of an [ECS cluster](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/clusters.html). | `opentelemetry-cluster` | No |
| `aws.ecs.container.arn` | string | The Amazon Resource Name (ARN) of an [ECS container instance](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ECS_instances.html). | `arn:aws:ecs:us-west-1:123456789123:container/32624152-9086-4f0e-acae-1a75b14fe4d9` | No |
| `aws.ecs.cluster.arn` | string | The ARN of an [ECS cluster](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/clusters.html). | `arn:aws:ecs:us-west-2:123456789123:cluster/my-cluster` | No |
Copy link

@hossain-rayhan hossain-rayhan Nov 17, 2020

Choose a reason for hiding this comment

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

When we get task metadata from ECS Task Metadata Endpoint, it gives us the ClusterName instead of ClusterARN for EC2 Launch Type. Can't we add one for aws.ecs.cluster.name?

Copy link
Contributor Author

@willarmiros willarmiros Nov 18, 2020

Choose a reason for hiding this comment

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

I originally had it as cluster.name but, since I was populating the ARN for nearly all other fields and consumers could easily derive cluster name from the ARN, I figured recording the cluster ARN made more sense. On Fargate we are provided the cluster ARN by default, and on EC2 launch type, we have the region and account ID from the task ARN, so you can always construct the cluster ARN like I do here.

All this being said, I can still add a new field for the cluster name if you need it. I just wanted to reduce the possibility for redundant info.

Choose a reason for hiding this comment

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

I guess very few customers care about the whole ARN. Most useful info here is the resource name (ClusterName or TaskID). It's also implies that all the exporters should write their logic to extract resource name from ARN. It's not only extracting the resource name, they need to do other checks before that. If resource attribute map does not have the name fields, extract them and apply. The logic is not super complex but extra work and check.
So, in a sense, we are blocking our customers if any specific exporter doesn't come with these logics.

Also, I am kind of convinced with the idea of less redundant info. I am OK to follow the experts opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed offline: We're ok with this as is for now, and in the future if customers ask for it we can add cluster name, task ID, etc to this spec.

| `aws.ecs.launchtype` | string enum | The [launch type](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/launch_types.html) for an ECS task. | `EC2`<br>`Fargate` | No |
| `aws.ecs.task.arn` | string | The ARN of an [ECS task definition](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task_definitions.html). | `arn:aws:ecs:us-west-1:123456789123:task/10838bed-421f-43ef-870a-f43feacbbb5b` | No |
| `aws.ecs.task.family` | string | The task definition family this task definition is a member of. | `opentelemetry-family` | No |