Add semantic conventions for the AWS SDK. by anuraaga · Pull Request #1095 · open-telemetry/opentelemetry-specification

anuraaga · 2020-10-15T08:56:12Z

Fixes #968 (by being the first instrumentation-specific instrumentation)

Changes

This defines semantic attributes for instrumentation of the AWS SDK. We have instrumentation in several languages and they fill in some traditional semantic fields with varying coverage. AWS has also identified fields in request / response that are valuable for monitoring and we think this is a good list, though will naturally grow more, of attributes to get meaningful information from AWS API calls without the problems of copying the entire request/response.

The attributes are sort of "denormalized" as they are intended to be extracted from API calls, with instructions for each, and some attributes are common among the calls. This seems easy to implement, but let me know how this structure looks. Particularly interested in @thisthat's advice in terms of the YAML representations.

Oberon00 · 2020-10-15T09:06:30Z

specification/trace/semantic_conventions/instrumentation/aws-sdk.md

+| Attribute | Type | Description | Examples |
+|---|---|---|---|
+|awssdk.service | string | The service name of the request, as returned by the AWS SDK | `DynamoDB`, `S3` |
+|awssdk.operation | string | The operation name of the request, as returned by the AWS SDK | `GetItem`, `PutObject` |


For new trace semantic conventions, please use the YAML format.

I can't find any documentation on how to define conventions with YAML can you point me at it? I just see our tool to automatically convert MD to YAML, which seems to make sense since I figure we want the readable representation?

https://github.com/open-telemetry/opentelemetry-specification/tree/master/semantic_conventions

There is no such tool, the tool converts YAML to MD not the other way round 😃 The docs are in https://github.com/open-telemetry/opentelemetry-specification/blob/master/semantic_conventions/syntax.md but I think it's easiest to look at an existing YAML file.

However, since I just saw #1054 is not merged yet, I think it should be OK to convert these conventions later.

Oh man, I completely misread that README and thought @thisthat was autogenerating those PRs!

It has always been hard manual work 😇 the first step at least, everything else is automatic 😉

thisthat

That's a lot of attributes! :) thank you for collecting them!
Going through it quickly, I noticed some style errors. I would suggest to follow the recommendation of @Oberon00 and start with the YAML definition and then, generate the tables.
In YAML, I would create a general awssdk semantic convention which contains
the service and operation attributes. Then, one semantic convention for each type that extends the general awssdk convention. Does it make sense? If you need further help, let me know :)

thisthat · 2020-10-15T10:02:25Z

specification/trace/semantic_conventions/instrumentation/aws-sdk.md

+| Attribute | Type | Description | Examples |
+|---|---|---|---|
+|awssdk.service | string | The service name of the request, as returned by the AWS SDK | `DynamoDB`, `S3` |
+|awssdk.operation | string | The operation name of the request, as returned by the AWS SDK | `GetItem`, `PutObject` |


It has always been hard manual work 😇 the first step at least, everything else is automatic 😉

thisthat · 2020-10-15T10:04:07Z

specification/trace/semantic_conventions/instrumentation/aws-sdk.md

+
+| Attribute | Type | Description | Examples |
+|---|---|---|---|
+|awssdk.service | string | The service name of the request, as returned by the AWS SDK | `DynamoDB`, `S3` |


thisthat · 2020-10-15T10:08:58Z

specification/trace/semantic_conventions/instrumentation/aws-sdk.md

+| `awssdk.global_secondary_indexes` | string | JSON-serialize the `GlobalSecondaryIndexes` request list field |
+| `awssdk.local_secondary_indexes` | string | JSON-serialize the `LocalSecondaryIndexes` request list field |
+| `awssdk.provisioned_throughput. | int | d_capacity_units` - Copy the `ProvisionedThroughput.ReadCapacityUnits` request parameter |
+| `awssdk.provisioned_throughput. | int | te_capacity_units` - Copy the `ProvisionedThroughput.ReadCapacityUnits` request parameter |


There is some strange errors here

anuraaga · 2020-10-15T10:43:46Z

Thanks I'll try to migrate! First impression was that it seems I always duplicate ID and name, I wonder if one can be made optional.

Oberon00 · 2020-10-15T11:47:07Z

You mean for enums? Yeah, I think defaulting value to id would make sense.

github-actions · 2020-10-24T03:27:21Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

anuraaga · 2020-10-24T03:28:24Z

Sorry for the delay on this, will be getting to the large YAML refactoring next week

semantic_conventions/trace/instrumentation/aws-sdk.yml

anuraaga · 2020-10-26T09:30:00Z

By the way, no need to review the yml yet I'm still working out generation issues to get to an MD file where I can confirm I wrote it correctly :) (I'm sure things like db.name are incorrect :)

github-actions · 2020-11-04T03:18:06Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

andrewhsu · 2020-11-06T00:20:19Z

at the metrics sig mtg today talked about this issue's staleness

Oberon00

General notes:

Blocking: You do not reference the new YAML convention in any md file. You need to add a new md file in the usual place with the <!-- semconv markers etc.
I think it's best practice to use quotes in multi-word YAML values, even if not required.
Please end the sentences with a period.

Oberon00 · 2020-11-06T14:54:49Z

semantic_conventions/trace/instrumentation/aws-sdk.yml

+    attributes:
+      - id: service
+        type: string
+        brief: The service name of the request, as returned by the AWS SDK


Suggested change

brief: The service name of the request, as returned by the AWS SDK

brief: "The name of the service to which a request is made, as returned by the AWS SDK."

etc

Oberon00 · 2020-11-06T14:55:07Z

semantic_conventions/trace/instrumentation/aws-sdk.yml

+  - id: awssdk
+    prefix: awssdk
+    brief: >
+      These conventions apply to operations using the AWS SDK. They map request or response parameters


Suggested change

These conventions apply to operations using the AWS SDK. They map request or response parameters

The `awssdk` conventions apply to operations using the AWS SDK. They map request or response parameters

Oberon00 · 2020-11-06T14:55:38Z

semantic_conventions/trace/instrumentation/aws-sdk.yml

+    brief: >
+      These conventions apply to operations using the AWS SDK. They map request or response parameters
+      in AWS SDK API calls to attributes on a Span. The conventions have been collected over time based
+      on feedback from AWS users of tracing and will continue to increase as new interesting conventions


Suggested change

on feedback from AWS users of tracing and will continue to increase as new interesting conventions

on feedback from AWS users of tracing and will continue to be extended as new interesting conventions

Oberon00 · 2020-11-06T14:55:57Z

semantic_conventions/trace/instrumentation/aws-sdk.yml

+      on feedback from AWS users of tracing and will continue to increase as new interesting conventions
+      are found.
+
+      Some descriptions are also provided for populating OpenTelemetry semantic conventions.


What does this mean?

Oberon00 · 2020-11-06T14:57:58Z

semantic_conventions/trace/instrumentation/aws-sdk.yml

+          - PutItem
+
+  - id: dynamodb.batchgetitem
+    brief: DynamoDB.BatchGetItem


Suggested change

brief: DynamoDB.BatchGetItem

brief: "Semantic conventions for the `BatchGetItem` operation of the DynamoDB service."

Same for all the similar briefs.

Oberon00 · 2020-11-06T14:59:02Z

semantic_conventions/trace/instrumentation/aws-sdk.yml

+    brief: DynamoDB.BatchGetItem
+    extends: awssdk
+    attributes:
+      - id: table_names


Please see #1115: If it can contain multiple strings, it should be an array. So either use table_name here or type: "string[]"

Oberon00 · 2020-11-06T15:01:05Z

semantic_conventions/trace/instrumentation/aws-sdk.yml

+    attributes:
+      - id: table_names
+        type: string
+        brief: Extract the keys of the `RequestItems` object field in the request


This is worded like an instruction for instrumentation writers. Since backend writers will also be interested in what this is, I suggest

Suggested change

brief: Extract the keys of the `RequestItems` object field in the request

brief: "The keys in the request's `RequestItems` object field."

I'm also not sure what this means (is it an object or string field now?) but I assume someone familiar with AWS would know.

Oberon00 · 2020-11-06T15:03:28Z

semantic_conventions/trace/instrumentation/aws-sdk.yml

+    brief: DynamoDB.DeleteItem
+    extends: awssdk
+    attributes:
+      - id: db.name


This will be dynamodb.deleteitem.db.name which is quite long. ~~If this was intentional, I'm OK with it though.~~ This looks fishy. If you generate the markdown, you can verify the attribute names are what you think.

Oberon00 · 2020-11-06T15:04:39Z

semantic_conventions/trace/instrumentation/aws-sdk.yml

+    brief: DynamoDB.DescribeTable
+    extends: awssdk
+    attributes:
+      - id: db.name


Is this different from dynamodb.deleteitem.db.name? Maybe you could extract some shared attributes and use them with constraints: include (as e.g. done for net in http).

github-actions · 2020-11-14T03:19:06Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2020-11-21T03:20:49Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

jgals · 2020-12-04T16:25:33Z

Hey @anuraaga, it looks like this issues was closed prematurely by the bot. Did you want to reopen it?

NathanielRN · 2021-07-26T23:28:12Z

Just in case anyone comes looking later, @anuraaga got all the comments addressed and merged in #1422 as a replacement for this PR 🥳

Add semantic conventions for the AWS SDK.

7f8fe28

anuraaga requested review from a team October 15, 2020 08:56

github-actions bot assigned arminru Oct 15, 2020

Oberon00 reviewed Oct 15, 2020

View reviewed changes

Lint

b124776

thisthat reviewed Oct 15, 2020

View reviewed changes

bogdandrutu closed this Oct 15, 2020

bogdandrutu reopened this Oct 15, 2020

arminru added area:semantic-conventions Related to semantic conventions release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:trace Related to the specification/trace directory labels Oct 16, 2020

github-actions bot added the Stale label Oct 24, 2020

github-actions bot removed the Stale label Oct 25, 2020

Anuraag Agrawal added 2 commits October 26, 2020 18:16

Initial MD -> YML

1410a31

Fix

aa3b3b3

anuraaga commented Oct 26, 2020

View reviewed changes

semantic_conventions/trace/instrumentation/aws-sdk.yml Show resolved Hide resolved

github-actions bot added the Stale label Nov 4, 2020

jmacd removed the Stale label Nov 6, 2020

Oberon00 requested changes Nov 6, 2020

View reviewed changes

github-actions bot added the Stale label Nov 14, 2020

github-actions bot closed this Nov 21, 2020

willarmiros mentioned this pull request Jan 27, 2021

Add AWS SDK Instrumentation open-telemetry/opentelemetry-js-contrib#323

Closed

anuraaga mentioned this pull request Feb 10, 2021

AWS SDK common and dynamodb conventions #1422

Merged

	\|awssdk.service \| string \| The service name of the request, as returned by the AWS SDK \| `DynamoDB`, `S3` \|
	\| `awssdk.service` \| string \| The service name of the request, as returned by the AWS SDK \| `DynamoDB`, `S3` \|

	brief: The service name of the request, as returned by the AWS SDK
	brief: "The name of the service to which a request is made, as returned by the AWS SDK."

	These conventions apply to operations using the AWS SDK. They map request or response parameters
	The `awssdk` conventions apply to operations using the AWS SDK. They map request or response parameters

	on feedback from AWS users of tracing and will continue to increase as new interesting conventions
	on feedback from AWS users of tracing and will continue to be extended as new interesting conventions

	brief: DynamoDB.BatchGetItem
	brief: "Semantic conventions for the `BatchGetItem` operation of the DynamoDB service."

	brief: Extract the keys of the `RequestItems` object field in the request
	brief: "The keys in the request's `RequestItems` object field."

Conversation

anuraaga commented Oct 15, 2020

Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thisthat left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anuraaga commented Oct 15, 2020

Uh oh!

Oberon00 commented Oct 15, 2020

Uh oh!

github-actions bot commented Oct 24, 2020

Uh oh!

anuraaga commented Oct 24, 2020

Uh oh!

Uh oh!

anuraaga commented Oct 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 4, 2020

Uh oh!

andrewhsu commented Nov 6, 2020

Uh oh!

Oberon00 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Oberon00 Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 14, 2020

Uh oh!

github-actions bot commented Nov 21, 2020

Uh oh!

jgals commented Dec 4, 2020

Uh oh!

NathanielRN commented Jul 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

anuraaga commented Oct 26, 2020 •

edited

Loading

Oberon00 Nov 6, 2020 •

edited

Loading