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

botocore: Add DynamoDB extension #735

Merged
merged 5 commits into from
Oct 18, 2021

Conversation

mariojonke
Copy link
Contributor

@mariojonke mariojonke commented Oct 13, 2021

Description

Adds an extension to extract additional request/response attributes for DynamoDB as defined in the semantic conventions for the AWS SDK. Depending on which DynamoDB operation is called via the AWS SDK, different attributes are extracted.

To follow the database semantic conventions, relevant connection and call-level attributes are also extracted. Those attributes are:

  • db.system: always 'dynamodb'
  • db.operation: corresponds to the called DynamoDB method/operation of the AWS SDK
  • net.peer.name: basically corresponds to 'dynamodb..amazonaws.com'

Removes the attribute aws.table_name as according to the AWS SDK spec, aws.dynamodb.table_names should be used. (According to the spec the attribute is now also a list of table names instead of a single table name.)

Type of change

  • Breaking change: breaks existing span attribute names to make them spec compliant

How Has This Been Tested?

Adds additional unit tests to verify that DynamoDB related attributes are extracted accordingly

Does This PR Require a Core Repo Change?

  • 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

* extract addtional DynamoDB specific attributes according to the spec
* move DynamoDB tests to separate test module
@mariojonke mariojonke requested a review from a team October 13, 2021 12:16
Copy link
Contributor

@ocelotl ocelotl left a comment

Choose a reason for hiding this comment

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

Minor license comment only. Hey, it seems like you know botocore pretty well, interested in becoming a codeowner?


_OPERATION_MAPPING = {
op.operation_name(): op
for op in globals().values()
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why use globals and a series of ifs if the list of classes can be known beforehand? Is it expected that this module will receive more classes at runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the idea is to calculate it automatically so that operations that are added later (not at runtime) might not accidentally be forgotten to be added to the _OPERATION_MAPPING

@mariojonke
Copy link
Contributor Author

added missing license info

Hey, it seems like you know botocore pretty well, interested in becoming a codeowner?

i guess i could do that

@ocelotl ocelotl merged commit 3058281 into open-telemetry:main Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants