Remove context.db from S3 spans: S3 isn't a database

[trentm]

S3 isn't a DB. It is specified to have span.type == "storage", not "db".
I believe context.db.* was added to the spec for S3 spans by accident (see below).
I propose that we (a) drop context.db.* from the spec for S3 spans and (b) possibly move the "S3" spec section out of "tracing-instrumentation-db.md" (either back to the AWS spec file, or a new "storage" spec file).

By comparison, our spec for other "storage" spans -- Azure Blob Storage, Azure Files, Azure Storage Table -- do not specify adding context.db.*.

Current (experimental) OTel semantic conventions for AWS SDK do not provide any details specific to S3 that we might want to align with.

Currently S3 context.db.instance is specified to be the target region. A better field for the target region is the ECS cloud.target.region field. We already specify span.context.destination.cloud.region for this. We just need to (a) get the intake API to use it and (b) move the ECS field to Stage 3 (as discussed here: elastic/ecs#1282). (Arguably we could drop context.db.instance = $region from the DynamoDB spec as well.)

Open questions

• Does dropping context.db.* for S3 spans impact APM UI? I don't think so, but I might have missed something.

History of "context.db" for S3 spans

I believe #420 may have accidentally added context.db to the spec for S3 as part of a refactor. That change refactored DynamoDB and S3 specs from "tracing-instrumentation-aws.md" to "specs/agents/tracing-instrumentation-db.md". In the process, context.db.* fields were added for S3... copying the values from the DynamoDB section:

| __**context.db._**__  |<hr/>|<hr/>|
|`_.instance`| e.g. `us-east-1` | The AWS region where the table is. |
|`_.statement`| - |  |
|`_.type`|`dynamodb`|

PR #451 followed up later to fix the copypasta:

| __**context.db._**__  |<hr/>|<hr/>|
|`_.instance`| e.g. `us-east-1` | The AWS region where the bucket is. |
|`_.statement`| - |  |
|`_.type`|`s3`|

From the discussion on #420, I don't see that there was specific intent to add context.db for S3 spans.

checklist

• May the instrumentation collect sensitive information, such as secrets or PII (ex. in headers)?
  • Yes
    • Add a section to the spec how agents should apply sanitization (such as sanitize_field_names)
  • No
    • Why?
  • n/a
• Create PR as draft
• Approval by at least one other agent
• Mark as Ready for Review (automatically requests reviews from all agents and PM via CODEOWNERS)
  • Remove PM from reviewers if impact on product is negligible
  • Remove agents from reviewers if the change is not relevant for them
• Approved by at least 2 agents + PM (if relevant)
• Merge after 7 days passed without objections
  To auto-merge the PR, add /schedule YYYY-MM-DD to the PR description.
• Create implementation issues through the meta issue template (this will automate issue creation for individual agents): Remove context.db from S3 spans: S3 isn't a database #696

[trentm]

Sylvain, Alex, this is a follow-up from the "Q7" discussions on #674 (comment) about how service.target.* is a special-case for S3 spans: type="storage" spans aren't currently well-defined in the general algorithm to infer service.target.* from other span fields.

[apmmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2022-09-30T05:27:00.929+0000

• Duration: 3 min 5 sec

[AlexanderWert]

LGTM

Does dropping context.db.* for S3 spans impact APM UI?

I don't think that's the case.

[SylvainJuge]

LGTM, thanks for cleaning this and doing spec archeology !