Incorrect result for memcached.operation_hit_ratio

[vijayaggarwal]

Component(s)

receiver/memcached

What happened?

Description

The memcached.operation_hit_ratio actually calculates miss ratio instead of hit ratio.

The function calculateHitRatio() (https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.92.0/receiver/memcachedreceiver/scraper.go#L175) expects arguments in the order (misses, hits) but is being passed arguments in the order (hits, misses). Thus, it actually calculates miss ratio instead of hit ratio.

Also, on a separate note, this metric calculates the ratio since the memcached instance start/restart, and hence the result changes on instance restart. This is counterintuitive in my opinion, as I would expect the metric to represent "recent" hit ratio. Since there is not universal definition of recent, I feel this metric should be deprecated and people should be encouraged to calculate the hit ratio from the memcached.operations metric as per their requirement.

Collector version

all recent versions

Environment information

No response

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

No response

[github-actions]

Pinging code owners:

• receiver/memcached: @djaglowski

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[djaglowski]

Thanks for reporting @vijayaggarwal. I'm in support of deprecating the metric.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• receiver/memcached: @djaglowski

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[kernelpanic77]

@djaglowski Can I take this up ?

[djaglowski]

Yes, thanks @kernelpanic77

[kernelpanic77]

@djaglowski Should we deprecate this metric or disable it in the metadata.yaml of memcachedreciever ?

[djaglowski]

@kernelpanic77 we should follow the process described here.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• receiver/memcached: @djaglowski

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• receiver/memcached: @djaglowski

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

This issue has been closed as inactive because it has been stale for 120 days with no activity.