What do you want to see in Dalli 4.x?

[petergoldstein]

I'm looking for any thoughts on what users would like to see in the Dalli 4.x version.

In my view Dalli 3.x is essentially a modernization release. Key changes in this version either include or will include:

• Switching to Github Actions
• Updating CI to reflect modern versions of Ruby VMs and memcached, and resolving any issues
• Removing the deprecated DalliStore
• Adding linting to CI and addressing various lints, to make the code more idiomatic and maintainable
• Enhancing current documentation
• Updating the rack session support to be compatible with recent versions of Rack/Rails (3.1.x)
• Changing the primary branch name in line with Github recommendations
• Getting SASL tests running in CI
• Significant refactoring of the code to make it more maintainable and to ensure edge cases are properly tested

When this agenda is complete Dalli should be free of meaningful technical debt and relatively straightforward to support for the near future.

That leaves the question of what's next - what should be in Dalli 4.x?

The most obvious choice would be adding support for the (relatively) new Memcache Meta protocol. The binary protocol that Dalli uses is now deprecated (although it's planned to be supported for the foreseeable future). Is this of interest?

Are there any other features or enhancements you'd like to see in Dalli? ActiveSupport::Notifications events? Implementation of a Probabilistic Hot Key Cache?

Thoughts are welcome. Thanks!

[petergoldstein]

A version of Dalli that supports the Meta protocol as an option can be found here - #879 . It will likely be released as 3.2.x.

While this version allows users to switch out the default binary protocol for Meta, it makes no API changes that allow Dalli users to take advantage of new Meta features. Exposing those features will require substantial extension of or changes to the ::Dalli::Client

[petergoldstein]

I've released this version as 3.2.0. Feedback is welcome.

[robertlaurin]

Following up from the issue here to document the feature request around instrumentation support.

I would like to push for the Dalli gem to adopt first party instrumentation support for OpenTelemetry (it never hurts to ask), however I understand that it is not always desirable to pick a specific standard in favour of providing a more generic interface for something like this.

Our current approach is to patch the request method in Dalli::Protocol::Binary so we can generate metrics for request latency, and so we can trace the request.

Here is an example of community maintained instrumentation for OpenTelemetry Ruby
https://github.com/open-telemetry/opentelemetry-ruby/blob/main/instrumentation/dalli/lib/opentelemetry/instrumentation/dalli/patches/server.rb#L13-L30

And a more visual demo of what the output looks like for a simple snippet of code like the following.

SimpleTracer.in_span('Parent span') do
  dalli.set('foo', 'bar')
  dalli.get('foo')

  dalli.set('foo2', 'bar2')
  dalli.get_multi('foo', 'foo2')

  SimpleTracer.in_span('sleep for a moment while I stop memcache') { sleep 5 }

  begin
    dalli.set('foo', 'bar')
  rescue => e
  end
end

Ideally the outcome of any work done in this effort would result in either a formally supported chokepoint in code that we could patch to capture telemetry data from the requests being made to memcached, or an interface where we could supply our own tracer/metrics reporter.

Let me know if you'd like for me to expand on anything above, or would like some contributions to spike out a poc.

[petergoldstein]

@robertlaurin This is helpful. A few questions:

1. Do we believe that the right level of "statement" is the API call to the Dalli::Client (which may not have a 1:1 correspondence to memcached API calls?). The choice here has significant design implications - it affects things like latency calculations, arguments size, and error handling.
2. Do we need to do any truncation / size limitation on args? If we're using Dalli::Client then get_multi may be problematic. We have users sending arrays of 500k keys, and there's no reason beyond some networking issues why that array size should be limited.
3. For caching cases (the most common usage of Dalli), the most valuable instrumentation is usually that which allows us to calculate cache hit ratios - especially with segmentation by key patterns or namespace. As I see it the OpenTelemetry metrics don't support that. Do you agree with that assessment? Do you have thoughts on how we might extend Dalli's telemetry to support that? Or do you believe that sort of metric is out of scope for this kind of instrumentation, and should be performed entirely by the caller if so desired?
4. There is an error flag in the telemetry result you posted, which I believe is generated when the call raises an exception. There are other "error" cases in Dalli that don't generate an exception (e.g. key doesn't exist for a get or replace, key already exists for an add, etc.) that don't generate an error per se, but depending on the scenario may indicate a problem with the call. Do you believe the instrumentation here needs to record that an "error" result was returned, or do you believe that's out of scope for the instrumentation and should be managed by the caller if needed?
5. Calls within a quiet or multi block are basically the same as those made outside one from the code path perspective. This is true both for the binary and the meta implementations. But the latency behavior and error handling is somewhat different. Latency will be especially different, as the call will not wait to read any response off the wire. This means that mixing latencies for quiet and non-quiet calls (for example, when calculating average latencies) will be badly misleading. Do you believe the telemetry needs to distinguish between these two types of calls? If so, do you think the quiet state should be tracked with a dedicated attribute passed to a tracer/metrics reporter?
6. The current OpenTelemetry approach - concatenating cmd and args into a single "statement" - seems to make a number of common use cases (calculating average latency over requests of a particular type, etc.) harder. Can you give a little insight as to the considerations that drove this design? Is there any reason that Dalli wouldn't produce a more granular output?
7. The get_multi behavior is essentially entirely different from all of the other Dalli interaction with memcached. Every other action is single request dispatched to a single memcached server followed by an immediate, but potentially optional, response. Inside a quiet block, all possible responses are simply discarded. The get_multi batches up a set of keys (potentially very large, O(100k+)), which are typically distributed over the entire set of configured servers, Requests are written in batches to each of the servers, and then read at a later time (not immediately after the request) when the socket for that servers notifies that it has response data. It's very unclear to me how this gets mapped to OpenTelemetry or something like it. Right now it looks like you're generating a telemetry event for each queried server, but as there's no single endpoint at the server level for such a request this is fairly tricky. I'm not sure whether this is the right approach, or whether this data is useful. If we aggregate at a higher level (Dalli::Client vs. Dalli::Protocol::* then we can't provide any network information, because that's not meaningful. Any thoughts from your side on how to best handle this case?

Happy to work through specs/PRs if you and your team have the bandwidth.

[petergoldstein]

@robertlaurin Any thoughts on the above?

[robertlaurin]

@petergoldstein Hey thanks for ping, the original notification got lost over the holidays. I'm going to circulate these questions with a few of my peers (who work more directly with memcached) so my responses reflect more than just my own opinions. Thanks for following up.

[petergoldstein]

Thanks @robertlaurin . I look forward to the responses.

[robertlaurin]

Sharing the verbatim feedback received from my colleague @fbogsany

1. My general philosophy is that client spans should be "measured" as close to the wire as possible. It can be useful to instrument the higher level API as well, if there can be significant and variable overhead or if there are many wire-level API calls hidden behind the higher-level API, but that should provide additional context as "internal spans" rather than acting as the client spans.
2. OpenTelemetry Ruby needs to support truncation and size limits on args. This should not be a Dalli responsibility. We currently support limiting the total number of attributes and the max length of an attribute value, but this only applies (or assumes, actually) string-valued attributes. Trimming array values is TBD.
3. I think cache hit ratios are an important metric. OpenTelemetry metrics could be leveraged to do that, but would require work in Dalli's instrumentation or in the client. Our preference is to record key accesses in trace spans and to defer analysis of key patterns or namespaces to the trace backend. We have done this in an ad-hoc fashion with BigQuery, for example, and have ideas for exposing this analysis in a more friendly UI. Other metrics we've found useful include: value size for reads & writes, and "hot" keys, along with request latency & counts.
4. Anything that indicates a likely problem should be marked as an error, regardless of whether an exception is raised. On the flipside, where exceptions are used for control flow, but aren't necessarily "real" problems, the span should not be marked as errored. We've seen this in the memcached gem, for example, where cache misses raise an exception. In some cases, it may be difficult to determine whether something should be reported as an error, since it depends on the client's use-case. In those cases, it's worth considering whether the client should have a way to modify the span status or should simply wrap the call in their own span. If you allow the client to override the status, you can mark it as status=error and allow the client to overwrite that with status=ok.
5. Yes, telemetry should distinguish these cases. A dedicated attribute is one way to do this. A suffix on the operation name (for spans) is another. I'd probably opt for the attribute. We have some helpers for HTTP and Redis instrumentation, to allow propagating additional attributes to their spans. We use this a lot for adding peer.service attributes, for example. Something similar could be appropriate here.
6. I disagree with the current OpenTelemetry approach. This was chosen assuming some kind of possibly complex "database statement", like a SQL query. For KV stores, I think a more granular approach makes sense. The kinds of analysis I want to do basically involve looking at the keys accessed or updated. I'd rather have those in an attribute that I can easily pick apart, rather than having to parse a db.statement.
7. I think get_multi benefits from instrumentation at the higher level and at the lower (per-server request) level. I think we'd want a single get_multi span that's kind=internal as a parent of the lower-level spans. The lower-level spans would not use the implicit OpenTelemetry Context mechanism. Instead, we'd use start_span and keep track of the span with the query state, finish ing it once the corresponding response has been read.

[BenSandeen]

I know this is a fairly old post, but adding support for ActiveSupport::Notifications would resolve the issue I'm running up against currently