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

Rename net.protocol.*, net.sock.family and net.transport attributes to align with ECS #3371

Closed
Tracked by #1012
trask opened this issue Apr 5, 2023 · 9 comments · Fixed by #3426
Closed
Tracked by #1012
Assignees
Labels
area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory

Comments

@trask
Copy link
Member

trask commented Apr 5, 2023

What are you trying to achieve?

Rename these remaining net.* attributes (the ones not covered in #3199) to align with ECS:

  • net.protocol.name -> network.protocol.name
  • net.protocol.version -> network.protocol.version
  • net.sock.family -> network.type
  • net.transport -> network.transport

Unlike the other ECS alignment proposals where we see benefits to the ECS modeling/naming, there's not really any good argument why OTel or ECS attributes are better here.

And so the question remains whether we should align going foward with the existing OpenTelemetry attributes or align with the existing ECS attributes.

@jamesmoessis
Copy link
Contributor

And so the question remains whether we should align going foward with the existing OpenTelemetry attributes or align with the existing ECS attributes

This is now decided that we will align with the ECS attributes, IIUC. From the OTEP:

Where feasible, the goal is to align the OTel SemConv attributes as close as possible with the existing, stable ECS fields

These fields seem to map 1-to-1 so I don't see any reason why we shouldn't go ahead with this.

@trask
Copy link
Member Author

trask commented Apr 18, 2023

A quick search show that only Java instrumentation is using net.protocol.name and net.protocol.version today.

net.transport (e.g. ip_tcp) and net.sock.family (e.g. inet4, inet6) are more widely emitted by instrumentation, but I suspect the reliance on these attributes for alerting and dashboards is low.

@trask
Copy link
Member Author

trask commented Apr 18, 2023

There are also the net.host.connection.* and net.host.carrier.* attributes, which we should group into this issue about whether to rename net.* to network.*.

I'm not aware of any OpenTelemetry instrumentation that has implemented the net.host.connection.* and net.host.carrier.* attributes.

@AlexanderWert is there anything on the ECS side similar to the OpenTelemetry net.host.connection.* and net.host.carrier.* attributes?

@trask
Copy link
Member Author

trask commented Apr 19, 2023

I'd like to proprose that we move forward with this rename

@Oberon00
Copy link
Member

Oberon00 commented Apr 20, 2023

net.sock.family -> network.type: This sounds different from net.transport. It sounds like it would be set to LTE or Ethernet, etc (something currently stored in net.host.connection.type). Can we come up with a better name (network.sock_family)?

existing ECS network.* attributes are important to some existing security use cases on the ECS side

Is this a good enough argument? We could add any missing pieces to our OTel conventions instead

since we are already breaking the most important net.* attributes on the OTel side (#3402),

This one is still not merged...

@trask
Copy link
Member Author

trask commented Apr 20, 2023

net.sock.family -> network.type: This sounds different from net.transport. It sounds like it would be set to LTE or Ethernet, etc (something currently stored in net.host.connection.type). Can we come up with a better name (network.sock_family)?

ya, this is a good point, we should definitely consider this 👍

existing ECS network.* attributes are important to some existing security use cases on the ECS side

Is this a good enough argument? We could add any missing pieces to our OTel conventions instead

the argument here is that renaming network.* to net.* would cause more problems on the ECS side than for us to rename the remaining net.* to network.* on the OpenTelemetry side (assuming #3402 is accepted)

@AlexanderWert
Copy link
Member

net.sock.family -> network.type: This sounds different from net.transport. It sounds like it would be set to LTE or Ethernet, etc (something currently stored in net.host.connection.type). Can we come up with a better name (network.sock_family)?

I think OTel's net.sock.family and ECS' network.type describe slightly different things (though related):

  • net.sock.family refers to Linux Address Families which is not tied to a specific OSI Model layer (e.g. IPv4 / IPV6 is OSI Layer 3, while Bluetooth is Layer 2).
  • network.type refers to the OSI Layer 3 explicitly as you can see in the ECS description.

So, as those are describing different things, how about having both (network.sock_family and network.type)?

Regarding the name network.type, actually, the name describes what it is: the type of the OSI network layer (Layer 3).

@trask
Copy link
Member Author

trask commented Apr 24, 2023

I think OTel's net.sock.family and ECS' network.type describe slightly different things (though related):

  • net.sock.family refers to Linux Address Families which is not tied to a specific OSI Model layer (e.g. IPv4 / IPV6 is OSI Layer 3, while Bluetooth is Layer 2).
  • network.type refers to the OSI Layer 3 explicitly as you can see in the ECS description.

So, as those are describing different things, how about having both (network.sock_family and network.type)?

Regarding the name network.type, actually, the name describes what it is: the type of the OSI network layer (Layer 3).

@Oberon00 @lmolkova do you think we need both at this point? and is it reasonable to consolidate on network.type? I've only seen instrumentation populating net.sock.family values of ipv4 and ipv6 so far (and we can add more coverage later?)

@trask
Copy link
Member Author

trask commented Apr 24, 2023

I've split the "socket family" vs OSI layer 3 "network type" discussion out into a separate issue: #3438

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 spec:trace Related to the specification/trace directory
5 participants