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 aws.ecs.task.revision semantic convention #1581

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ release.

- Add details for filling semantic conventions for AWS Lambda ([#1442](https://github.com/open-telemetry/opentelemetry-specification/pull/1442))
- Update semantic conventions to distinguish between int and double ([#1550](https://github.com/open-telemetry/opentelemetry-specification/pull/1550))
- Add semantic convention for AWS ECS task revision ([#1581](https://github.com/open-telemetry/opentelemetry-specification/pull/1581))

### Compatibility

Expand Down
5 changes: 5 additions & 0 deletions semantic_conventions/resource/cloud_provider/aws/ecs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,9 @@ groups:
brief: >
The task definition family this task definition is a member of.
examples: ['opentelemetry-family']
- id: task.revision
type: string
brief: >
The revision for this task definition.
examples: ["8", "26"]
Comment on lines +39 to +42
Copy link
Member

Choose a reason for hiding this comment

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

If these are integer counters that get incremented for each revision, please use the integer type.
If the revision could be non-integer as well (e.g. "8b"), please add an example to make this clear.
Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

The endpoint returns a string but that string can be usually (maybe always?) parsed as an integer. AWS' docs only exemplify that the type returned is a string but they don't clarify if it's always an integer.

I think it's better to use a string in case something like 8b can be returned by the endpoint (either now or in the future), but I don't feel comfortable adding it as an example since I have not seen that happen in practice and no similar example exists in the docs.

Maybe @anuraaga or @rakyll can shed some light into this?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should stick with string if that's the type in the API docs I think

Copy link
Member

Choose a reason for hiding this comment

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

Alright! If we can't tell if it could be any string other than one resembling an integer I'm fine with also keeping the examples as is.


Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
| `aws.ecs.launchtype` | string | The [launch type](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/launch_types.html) for an ECS task. | `ec2`; `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 |
| `aws.ecs.task.revision` | string | The revision for this task definition. | `8`; `26` | No |

`aws.ecs.launchtype` MUST be one of the following:

Expand Down