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

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Mar 26, 2021

Changes

Add semantic convention for an ECS task definition revision, described in the AWS ECS Task metadata endpoint documentation as

The revision of the Amazon ECS task definition for the task.

Follow up to open-telemetry/opentelemetry-collector-contrib#2814.

@carlosalberto
Copy link
Contributor

@rakyll @anuraaga please take a look.

Comment on lines +39 to +42
type: string
brief: >
The revision for this task definition.
examples: ["8", "26"]
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.

@arminru arminru added the area:semantic-conventions Related to semantic conventions label Mar 26, 2021
@carlosalberto carlosalberto merged commit 7aad901 into open-telemetry:main Mar 30, 2021
@mx-psi mx-psi deleted the mx-psi/aws-ecs-task-revision branch March 30, 2021 15:36
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants