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

BREAKING: Rename remaining network attributes from net.* to network.* and align definitions with ECS #3426

Merged
merged 25 commits into from
May 9, 2023

Conversation

trask
Copy link
Member

@trask trask commented Apr 21, 2023

Fixes #3371
Fixes #3438

Changes

Introduce:

(note: ECS defines both network.transport and network.type only in terms of OSI, but we expand the definition here to be more general)

Remove net.transport and net.sock.family.

Rename:

  • net.protocol.* --> network.protocol.*
  • net.host.connection.* --> network.connection.*
  • net.host.carrier.* --> network.carrier.*

Summary of benefits

More precise definition of network.transport and network.type in terms of OSI model where applicable.

These attributes are used in ECS security use cases, so the cost of breaking these on the ECS side is higher, while these remaining net.* attributes are not heavily used in OpenTelemetry instrumentation (and we can bundle this into the transition plan for the larger breaking changes).

We can bring in the remaining ECS network.* attributes without causing ECS more breakage.

@trask trask force-pushed the rename-network-attrs branch 2 times, most recently from 0f1bfc7 to c1133b8 Compare April 21, 2023 03:31
@trask trask changed the title Rename network attributes from net to network Rename renaming network attributes from net to network Apr 21, 2023
@trask trask marked this pull request as ready for review April 21, 2023 03:37
@trask trask requested review from a team April 21, 2023 03:37
@reyang reyang added the area:semantic-conventions Related to semantic conventions label Apr 21, 2023
specification/trace/semantic_conventions/database.md Outdated Show resolved Hide resolved
specification/trace/semantic_conventions/database.md Outdated Show resolved Hide resolved
specification/trace/semantic_conventions/database.md Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member

tigrannajaryan commented Apr 24, 2023

I wonder if this impacts our ebpf implementation.

@open-telemetry/ebpf-maintainers @open-telemetry/ebpf-approvers can you comment? Is this a breaking change from your perspective?

@jmw51798
Copy link

This is not a breaking change for opentelemetry-ebpf because it does not currently emit any metrics defined in the opentelemetry-specification (although that should probably change). Instead, it uses historical metric names documented here: https://github.com/open-telemetry/opentelemetry-ebpf/blob/main/docs/metrics/metrics.yaml.

schemas/1.21.0 Outdated Show resolved Hide resolved
@jsuereth
Copy link
Contributor

jsuereth commented May 1, 2023

After a long discussion, +1. I feel like this really clarifies some very confusing x-layer attributes we had previously.

@Oberon00
Copy link
Member

Oberon00 commented May 4, 2023

@jsuereth

x-layer attributes

Can you clarify what you mean by this?

@trask
Copy link
Member Author

trask commented May 4, 2023

@jsuereth

x-layer attributes

Can you clarify what you mean by this?

I believe what he liked was that the attributes are now defined in terms of OSI layers.

@trask
Copy link
Member Author

trask commented May 9, 2023

don't merge yet, I need to double check the merge conflicts...

@trask
Copy link
Member Author

trask commented May 9, 2023

good to go!

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 semconv:database semconv:HTTP semconv:messaging spec:metrics Related to the specification/metrics directory spec:trace Related to the specification/trace directory
Projects
None yet