Revisit system network metrics attributes

[dmitryax]

Context

This issue is a follow-up to https://github.com/open-telemetry/semantic-conventions/pull/89/files#r1299769832. We need to take the opportunity of the attribute renaming required for #51 and choose names that inspire full confidence. The goal is to open a discussion and reach consensus on attribute names that minimize the risk of future changes.

All Network Metrics Attributes (Old Name / Current Name):

• device -> system.device
• direction -> system.network.direction
• protocol -> network.protocol
• state -> system.network.state

We should revisit each of these attributes within the scope of this issue. If you have any ideas or suggestions, please comment. The following are the main concerns we should address:

protocol / network.protocol

We picked network.protocol because it's generic and can be applicable to other metrics. It's listed in https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/attributes.md#network-attributes. We should rename it to network.protocol.name then.

state / system.network.state

While it currently sounds like a state of the network, in the OTel collector host metrics receiver, it represents connection status (e.g., "CLOSED," "LISTEN," ESTABLISHED," etc.). I think we should rename it to network.connection.status and add it to https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/attributes.md#network-attributes.

Sub-issues (added by @mx-psi):

• Rename device to system.device
  • Added network.interface.name
• Rename direction to system.network.direction
  • Renamed to network.io.direction with values transmit/receive
• Rename protocol to network.protocol or network.protocol.name
  • Renamed to network.protocol.name
• Rename state to system.network.state
• Rename system.cpu.logical_number to host.cpu.logical_number

[joaopgrassi]

The other thing that came up was about the system.cpu.logical_number. From @ChrsMark comment on #89:

Don't want to block the merge of this but shouldn't the system.cpu.logical_number be under the host. namespace since it's a resource attribute? So it would be host.cpu.logical_number or host.cpu.id similar to what we have for the host.id?

So if we revisit the attributes, we need to look at this one as well.

[dmitryax]

The other thing that came up was about the system.cpu.logical_number

Thanks. That should go to another cpu related issue

[mx-psi]

I am moving this to blocked since I want to have a clear picture of #394, since I think questions like #330 (comment) may arise here

[mx-psi]

Moving back to Todo, we have treated #394 as a MUST, but it is still unclear and would help if it's resolved to make progress here cc @joaopgrassi

[ChrsMark]

Bumping this up. I see that most of the initial concerns should now be covered:

network.direction

It has been renamed to network.io.direction with values transmit/receive.

system.device

Still system.device.

network.protocol

It has been renamed to network.protocol.name.

system.network.state

Still system.network.state

From the above I only see the system.network.state as possibly pending to change to system.network.status as proposed by @dmitryax at #308. @open-telemetry/semconv-system-approvers what do you think about this?

Otherwise we can consider it as completed.

[ChrsMark]

One question that pops up is if we should use the system.device attribute or just introduce a new one i.e. network.interface.name. This is also related to #1427 (comment) and #1408 (comment).

[ChrsMark]

We discussed about system.device attribute for network metrics in K8s SemConv SIG meeting, Oct 16th. 2 questions occurred:

1. should we use system.device for network metrics or we should introduce a new one for network interface specifically, i.e. network.interface.name.
2. Should the decided attribute be shared between hostmetrics receiver and kubeletstats receiver? Or for k8s we should have a k8s.* specific one?

/cc @open-telemetry/semconv-system-approvers @open-telemetry/semconv-k8s-approvers

[ChrsMark]

We discussed about system.device attribute for network metrics in K8s SemConv SIG meeting, Oct 16th. 2 questions occurred:

1. should we use system.device for network metrics or we should introduce a new one for network interface specifically, i.e. network.interface.name.
2. Should the decided attribute be shared between hostmetrics receiver and kubeletstats receiver? Or for k8s we should have a k8s.* specific one?

/cc @open-telemetry/semconv-system-approvers @open-telemetry/semconv-k8s-approvers

Systems WG discussed that in Oct 17th meeting. We agreed that a netowork.interface.* attribute is more appropriate for network metrics. I'm going to file a PR to apply this decision.

[ChrsMark]

Bumping up the remaining item:

From the above I only see the system.network.state as possibly pending to change to system.network.status as proposed by @dmitryax at #308. @open-telemetry/semconv-system-approvers what do you think about this?

[braydonk]

I personally don't feel strongly enough to fight much for it, but given the choice would agree that status is a better name than state. Upon further thinking I actually like state better. See my next comment.

[trask]

it looks like we have some of both currently:

status

• deployment.status
• k8s.container.status.last_terminated_reason
• metric.hw.status
• signalr.connection.status
• system.process.status
• system.processes.status
• test.case.result.status
• test.suite.run.status

state

• android.state
• container.cpu.state
• db.client.connection.state
• db.client.connections.state
• db.statement
• http.connection.state
• hw.state
• ios.state
• jvm.thread.state
• linux.memory.slab.state
• nodejs.eventloop.state
• process.cpu.state
• system.cpu.state
• system.filesystem.state
• system.memory.state
• system.network.state
• system.paging.state

do you think there are specific times when it makes sense to use one vs the other? or could we make a recommendation on generally preferring one of them?

[braydonk]

If I were to bikeshed state vs status (to a probably unreasonable degree) I would probably think:

• state would be representing being in one of a set of possible states at a given point
• status would be referring to a level of quality, i.e. how good/bad something is doing, perhaps from an enum of possible statuses but they would be on a relative scale to each other

I don't know how well those semantics would hold up in all scenarios though.

In System Semconv, I am usually biased towards following whatever verbiage is used in common reference to whatever the metric/attribute refers to. So, contrary to my last comment, I may have changed my mind about network.state. When I call nestat -a, each line has this:

Proto Recv-Q Send-Q Local Address           Foreign Address         State      
tcp        0      0 localhost:xyz           0.0.0.0:*               LISTEN 

On Windows netstat says the same thing, and so does SysInternals TCPView.

For a similar reason process.status is probably best to be renamed to process.state, given that /proc/[pid]/status calls it that and the equivalent in Win32 APIs calls it ExecutionState.

I feel like the generic semantics between state and status that I mentioned above stands up pretty well on its own, but it's probably stronger to make sure that the names of metrics/attributes match the contextual verbiage used even when it violates those generic semantics.

[ChrsMark]

Correcting myself from the previous comment. The suggestion is to rename system.network.state to network.connection.status.

In the hostmetrics receiver, it indeed represents the connection status (e.g., "CLOSED," "LISTEN," ESTABLISHED," etc.).

I still have no strong opinions here but if there is no support for this change we can move on and closed this issue as completed. @braydonk @dmitryax anything to add?

[braydonk]

My vote is to keep the word state, but maybe rename it to network.connection.state.

[ChrsMark]

My vote is to keep the word state, but maybe rename it to network.connection.state.

Sent a PR: #308 (comment)