opentelemetry-instrumentation-system-metrics: remove direction attribute

[codeboten]

Following the spec change here: open-telemetry/opentelemetry-specification#2617

The attribute direction is being replaced by the addition of the direction to the metric name.

[dbgoytia]

Hey @codeboten ! I'd like to contribute to this problem, where do these changes need to be applied? 🤔.

[ocelotl]

Hey @codeboten ! I'd like to contribute to this problem, where do these changes need to be applied? thinking.

Here. Thanks for your interest, I'll assign you to this issue 🙂

[petertamuk]

Hi @dbgoytia, wanted to check in and see if this issue was being actively worked on. I'm looking to start contributing and this seemed like a good place to start if you haven't found the time to make much progress on it. Thanks

[dbgoytia]

Hi @petertamuk ! I'm currently still working on it but happy to coauthor the solution if you would like 😊

[dbgoytia]

Actually go with it @petertamuk !! Happy to see the solution you give and if you're open to it let me know if I can be of any help

[petertamuk]

@dbgoytia Appreciate it. @codeboten I believe I have a handle on the issue and a good idea around the fix. Just wanted to confer on the new metric names.

system.disk.io -> (system.disk.read_io, system.disk.write_io)
system.disk.operations -> (system.disk.read_operations, system.disk.write_operations)
system.disk.time -> (system.disk.read_time, system.disk.write_time)
system.network.dropped.packets -> (system.network.dropped.packets_in, system.network.dropped.packets_out)
system.network.packets -> (system.network.packets_in, system.network.packets_out)
system.network.errors -> (system.network.errors_in, system.network.errors_out)
system.network.io -> (system.network.io_in, system.network.io_out)

Additionally, does the scope of this change include updates to DiskIO and NetIO?
ie
["read_count","write_count","read_bytes", "write_bytes","read_time","write_time","read_merged_count","write_merged_count",]
->
["count","bytes","time","merged_count",]

and

["dropin","dropout","packets_sent","packets_recv","errin", "errout","bytes_sent","bytes_recv",]
->
["drop","packets","err","bytes",]

[codeboten]

please look at the specification for the metric names: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/system-metrics.md#systemdisk---disk-controller-metrics

[petertamuk]

@codeboten Thanks for pointing that out. Just wanted to clarify on the spec.

I dont see system.disk.time however system.disk.operation_time (deprecated) is present. Should these be treated as the same?

Additionally i dont see the metric system.disk.merged (deprecated) and spec is looking for system.disk.merged.read, system.disk.merged.write.

Any suggestions would be appreciated. Thanks

[codeboten]

It looks like there's a typo in the method below which is looking at the configuration for system.disk.time when it should probably look at the configuration for system.disk.merged:

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/__init__.py

Lines 469 to 485 in 7c75b38

	def _get_system_disk_merged(
	self, options: CallbackOptions
	) -> Iterable[Observation]:
	"""Observer callback for disk merged operations"""
	# FIXME The units in the spec is 1, it seems like it should be
	# operations or the value type should be Double
	for device, counters in psutil.disk_io_counters(perdisk=True).items():
	for metric in self._config["system.disk.time"]:
	if hasattr(counters, f"{metric}_merged_count"):
	self._system_disk_merged_labels["device"] = device
	self._system_disk_merged_labels["direction"] = metric
	yield Observation(
	getattr(counters, f"{metric}_merged_count"),
	self._system_disk_merged_labels.copy(),
	)

I believe the metric for system.disk.time is the same as operation_time, so that would be the right one to use.

[petertamuk]

I submitted a PR with my changes. Hopefully the way I went about it is inline with what you thinking. Please let me know if there is anything I could have done better. Happy to make updates if needed.