Agent Breakdown Metrics

[felixbarny]

Description of the issue

To facilitate breakdown graphs (#70), agents should track metrics for the self-time of spans grouped by their span.type, as well as the duration for the transactions.

Example documents:

{
  "metricset": {
    "timestamp": 1555603735414000,
    "transaction": {
      "name": "test transaction",
      "type": "request"
    },
    "span": {
      "type": "db",
      "subtype": "mysql"
    },
    "samples": {
      "span.self_time.count": { "value": 100 },
      "span.self_time.sum.us":   { "value": 100000 }
    }
  }
}
{
  "metricset": {
    "timestamp": 1555603735414000,
    "transaction": {
      "name": "test transaction",
      "type": "request"
    },
    "span": {
      "type": "app"
    },
    "samples": {
      "span.self_time.count": { "value": 10 },
      "span.self_time.sum.us":   { "value": 200000 }
    }
  }
}
{
  "metricset": {
    "timestamp": 1555603735414000,
    "transaction": {
      "name": "test transaction",
      "type": "request"
    },
    "samples": {
      "transaction.duration.count":  { "value": 10 },
      "transaction.duration.sum.us":    { "value": 300000 },
      "transaction.breakdown.count": { "value": 10 },
    }
  }
}

For more details, background and explanation, see this concept: https://docs.google.com/document/d/1-_LuC9zhmva0VvLgtI0KcHuLzNztPHbcM0ZdlcPUl64

What we are voting on

Tick your box if the concept looks good to you in general and if you think we can proceed. The details can be adjusted along the way. I'll create implementation issues if we agree to proceed with this concept.

Vote

Agent	Yes	No	Indifferent	N/A	Link to agent issue
.NET					elastic/apm-agent-dotnet#227
Go					elastic/apm-agent-go#528
Java					elastic/apm-agent-java#556
Node.js					elastic/apm-agent-nodejs#1064
Python					elastic/apm-agent-python#479
Ruby					elastic/apm-agent-ruby#420
RUM					elastic/apm-agent-rum-js#289

[roncohen]

Look like we have good alignment with regard to the existing documents we create. However, this seems new? ;)

"span": {
  "type": "transaction"
 },

[felixbarny]

More details on the span.type=transaction here: https://docs.google.com/document/d/1-_LuC9zhmva0VvLgtI0KcHuLzNztPHbcM0ZdlcPUl64#heading=h.6rintxaaojnh and here: https://docs.google.com/document/d/1-_LuC9zhmva0VvLgtI0KcHuLzNztPHbcM0ZdlcPUl64/edit#heading=h.cs6w4ad6jo6c

[jalvz]

Dear @elastic/apm-agent-devs ,
Since server needs to provide explicit support for this (eg for UI to do aggregations based on some of these fields), and we target 7.3, and we don't want breaking/complicated changes after that; I'd much appreciate if you get on ticking boxes and/or amending the proposal.

Truly yours,

Juan

[felixbarny]

There's actually one change we may need to do which is to change transaction.duration.sum to transaction.duration.sum.us and span.self_time.sum to span.self_time.sum.us. That also means reporting in µs instead of ms.

@axw As I'll be on vacation for the next two weeks, could you help to make sure we get to a decision on that?

[felixbarny]

@elastic/apm-agent-devs PSA: I have updated the description to reflect the proposed changes ^

[jalvz]

will these fields be required?

[axw]

@jalvz which fields? The server can't require any of them, as that'll break older agents.

We could do some kind of conditional requirement. These are the expectations:

• span.self_time.count and span.self_time.sum.us are either both present, or both missing
• likewise for transaction.duration.count, transaction.duration.sum.us

transaction.breakdown.count is currently only present along with those last two fields, but I don't know if we should enforce that at the schema level. We might later want to relax things, e.g. we could optimise storage slightly by assuming transaction.breakdown.count == transaction.duration.count if the former is not specified.

For all of these metrics, transaction.name and transaction.type must be present. For the span.self_time metrics, span.type must also be present, and span.subtype may be present.

[simitt]

@felixbarny just raised concerns about the newly added properties on the Intake API for metricsets https://github.com/elastic/apm-server/blob/1ac64d62933d62d965efb98e38c088fee9ae4d8f/docs/spec/metricsets/metricset.json#L23...L88.
The properties are actually sent up within the already existing property spec, e.g.

{
  "metricset": {
    "timestamp": 1555603735414000,
    "transaction": {
      "name": "test transaction",
      "type": "request"
    },
    "span": {
      "type": "db",
      "subtype": "mysql"
    },
    "samples": {
      "span.self_time.count": { "value": 100 },
      "span.self_time.sum.us":   { "value": 100000 }
    }
  }
}

Therefore the added properties will not be used.

Referring to #78 (comment) we need to decide if we should remove the currently unused properties from the Intake API again and not do any required/conditionally required validation on metrics, or change the definition and add the requirements to the Intake API. At the moment, no requirement validations are implemented for those fields.

I suggest to not apply those validations and thus remove the additional fields on the Intake API, as applying validations for the next version would technically be a breaking change.

[felixbarny]

I suggest to not apply those validations and thus remove the additional fields on the Intake API

++

[axw]

Removing the properties/having no validation sounds fine to me.