buildbarn / bb-storage

Storage daemon, capable of storing data for the Remote Execution protocol
Apache License 2.0
137 stars 91 forks source link

blobstore: Add `metrics_tag` config field #205

Open minor-fixes opened 4 months ago

minor-fixes commented 4 months ago

This change adds a single string field to the base blobstore config message, the value of which gets propagated to a tag value on exported metrics for the corresponding backend instance.

In particular, this change allows for attaching different tags on separate uses of a single "label" backend in order to collect metrics specific to a particular instantiation. Without this change, all accesses to the referenced backend shared one combined set of metrics; with this change, it's possible to set separate metrics_tag values on each instantiation and collect separate metrics, as well as tagging the underlying backend and grabbing aggregate metrics as well.

TODO before marking non-draft:

Tested: see comment below

minor-fixes commented 4 months ago

With the config attached below, I started a bb_storage instance, and issued 7 RPCs:

The counts for the respective tags were:

I wouldn't mind adding an appropriate test though; any ideas on a better testing approach?


Config:

local SmallCache = function(tag) {
  metricsTag: tag,
  'local': {
    keyLocationMapInMemory: { entries: 1000 },
    keyLocationMapMaximumGetAttempts: 8,
    keyLocationMapMaximumPutAttempts: 32,
    oldBlocks: 2,
    currentBlocks: 1,
    newBlocks: 1,
    blocksInMemory: {
      blockSizeBytes: 10 * 1000 * 1000,
    },
  },
};

{
  executeAuthorizer: { allow: {} },
  grpcServers: [
    {
      listenAddresses: ["127.0.0.1:8980"],
      authenticationPolicy: { allow: {} },
    },
  ],
  global: {
    diagnosticsHttpServer: {
      httpServers: [{ listen_addresses: ["127.0.0.1:9980"], authenticationPolicy: { allow: {} } }],
      enablePrometheus: true,
    },
  },
  contentAddressableStorage: {
    getAuthorizer: { allow: {} },
    putAuthorizer: { allow: {} },
    findMissingAuthorizer: { allow: {} },
    backend: {
      withLabels: {
        backend: {
          metricsTag: 'top',
          demultiplexing: {
            instanceNamePrefixes: {
              '': {
                backend: {
                  metricsTag: 'read_cached',
                  readCaching: {
                    slow: { label: 'slow', metricsTag: 'slow_cached' },
                    fast: { label: 'fast', metricsTag: 'fast_cached' },
                    replicator: { 'local': {} },
                  },
                },
              },
              'slow': {
                backend: {
                  label: 'slow',
                  metricsTag: 'slow_direct',
                },
              },
              'fast': {
                backend: {
                  label: 'fast',
                  metricsTag: 'fast_direct',
                },
              },
            },
          },
        },
        labels: {
          'fast': SmallCache('fast_underlying'),
          'slow': SmallCache('slow_underlying'),
        },
      },
    },
  },
}
EdSchouten commented 3 months ago

Instead of actually having a separate metrics_tag, what are your thoughts on reusing the existing with_label / label facility? As in, make it so that using with_label causes a label="..." to be added to any BlobAccess underneath.