[APM] Show `error.exception[0].attributes` object in Metadata tab on Errors detail page

[formgeist]

Blocked: waiting from feedback from node agent devs. Additionally the opbeans-node application should send chained exceptions with "attributes" prop for the UI to reliably implement this

Summary

Add error.exception.attributes to the Metadata tab for the Errors detail page. Background: https://github.com/elastic/apm-agent-nodejs/issues/358

Design

Adding error.exception.attributes as a section in Metadata tab: note: the actual data in the screenshot is not correct

Possible implementation

Add a new section to the section definitions:

export const exceptionAttributes: Section = {
  key: 'exception.attributes',
  label: i18n.translate('xpack.apm.metadataTable.section.exceptionAttributesLabel', {
    defaultMessage: 'Exception attributes'
  })
};

Add the new exceptionAttributes in the metadata for errors

[elasticmachine]

Pinging @elastic/apm-ui

[sorenlouv]

@formgeist Was it agreed not to copy it over to tags or custom?

[formgeist]

AFAIK we don't have want to add them to tags or custom, since it's already part of the payload and should just be displayed if something is added to the object, as discussed in the previous issue.

[sorenlouv]

From what I understand from @beniwohli said he already copies them over to custom.

[formgeist]

We shouldn't add anything to custom unless it's something the users themselves define. Thought that's been the rule since we changed the contextual data way back.

[beniwohli]

To be clear, I only do this for logs, not for errors/exceptions. The reason is that we can't differentiate between user-provided and default attributes on the LogRecord instance.

[formgeist]

^^ Spoke to @beniwohli on Zoom about the Python use-case, and it didn't seem like the two were specifically related.

@sqren Remind me, don't we already have checks whether i.e. the exception or log stacktraces are there in the data, and hide and show those contents/tabs depending on the check? In my mind we could something similar; if there's anything in the attributes object, we show a tab for it - we hide it if it's empty. Then it's useful for those users who do use attributes and less of a concern for the ones that don't.

[sorenlouv]

Remind me, don't we already have checks whether i.e. the exception or log stacktraces are there in the data, and hide and show those contents/tabs depending on the check?

We currently conditionally show the stacktraces for exception/log, and the top-level fields from context as tabs, so these fields:

• error.exception.stacktrace
• error.log.stacktrace
• error.context.*

It feels a little odd to add error.exception.attributes to the top level as well. Perhaps it's better to have an "Exceptions" tab, that contains the exception stacktrace and attributes.

[formgeist]

Yeah, sounds similar to this https://github.com/elastic/kibana/issues/18347 which I issued a while ago. I'll look into both cases and make a design proposal.

[sorenlouv]

@formgeist This sounds like a low-hanging fruit if we simply display "error.exception.attributes" in the metadata. WDYT?

[formgeist]

I like it - I think that's a great quick implementation

[sorenlouv]

Cool, updated.

[sorenlouv]

@formgeist btw. I've been thinking whether the order we have for the sections in the metadata tab makes sense. Seems a little random:

Errors: https://github.com/elastic/kibana/blob/5accc3c315994ef8f5256f1f8e86d0bd066bb58d/x-pack/legacy/plugins/apm/public/components/shared/MetadataTable/ErrorMetadata/sections.ts#L22-L34

Transaction: https://github.com/elastic/kibana/blob/5accc3c315994ef8f5256f1f8e86d0bd066bb58d/x-pack/legacy/plugins/apm/public/components/shared/MetadataTable/TransactionMetadata/sections.ts#L24-L38

Spans: https://github.com/elastic/kibana/blob/5accc3c315994ef8f5256f1f8e86d0bd066bb58d/x-pack/legacy/plugins/apm/public/components/shared/MetadataTable/SpanMetadata/sections.ts#L17-L24

[formgeist]

@sqren Yeah, I've spotted that too since we added the transaction and error info to the metadata table. Been meaning to bring it up, so glad you are too 🙂

I think first off we want to show the Labels at the top. This was to enhance discovery for those who are using them and also for those who might be interested in using the feature. For Errors and Transactions I think the order is reasonably consistent which is good. For Spans I'd change the order to;

• Labels
• Span
• Transaction
• Trace
• Service
• Agent

So in reverse order of their hierarchy, so they're more interconnected. WDYT?

[sorenlouv]

I think that sounds great. Will you make an issue (or perhaps a PR) with the proposed order?

[formgeist]

@sqren Sure, I'll open a PR 🤓

[formgeist]

Opened https://github.com/elastic/kibana/pull/49313 for the reordering

[dgieselaar]

error.exception is an array, not an object. Do we have to iterate over all the exceptions and then show attributes for each, or can we assume that if there are attributes, they will always be in the first entry of error.exception? @beniwohli do you know perhaps?

[beniwohli]

In the intake API, error.exception is an object:

https://github.com/elastic/apm-server/blob/edeb0dd9a3d0d287a0c9fccadb614ba40d25953f/docs/spec/errors/error.json#L52-L72

Not sure how exactly that gets translated into a list in the ES document. I assume it has something to do with the transformation that the APM Server does for chained exceptions. IIRC, @jalvz worked on that, so he might know more.

[sorenlouv]

Yes, it's an object in the intake, but an array in Elasticsearch which is what matters to the UI. I don't think this is a problem - we just need to figure out whether to display one or all.

Since support for chained exceptions is landing soon I think it makes most sense to show attributes for every exception in the list. Either in the metadata tab as suggested or as part of the stacktrace view.

I initially suggested it be on the metadata tab because it seemed easier but I don't think there is a straightforward way to cherry pick a single property attributes from a list of objects. You could ofc add support for this if you think it's worth it :)

[jalvz]

Correct: object in the intake, array in ES because of the chained causes transformation.

[dgieselaar]

@sqren Kind of forgot what the status was on this. The title is now:

Show error.exception[0].attributes but Since support for chained exceptions is landing soon I think it makes most sense to show attributes for every exception in the list.

implies that it should be shown for all the exceptions. Could you clarify?

Also, does any of the opbeans apps generate chained exceptions?

[beniwohli]

opbeans-python does (it should appear as ZeroDivisionError IIRC), but we don't support attributes on exceptions. I believe that was kind of a NodeJS specific thing

[sorenlouv]

I believe that was kind of a NodeJS specific thing

If this is a Node.js specific feature, let's park this until we have some more information from Node agent devs.

[axw]

The Go agent does capture exception attributes. Here's the error field from a sample document:

{
  "exception": [
    {
      "stacktrace": [
        {
          "library_frame": true,
          "exclude_from_grouping": false,
          "abs_path": "/home/andrew/go/src/go.elastic.co/apm/gocontext.go",
          "filename": "gocontext.go",
          "line": {
            "number": 131
          },
          "module": "go.elastic.co/apm",
          "function": "CaptureError"
        },
        {
          "exclude_from_grouping": false,
          "filename": "foo.go",
          "abs_path": "/tmp/foo.go",
          "line": {
            "number": 17
          },
          "module": "main",
          "function": "main"
        },
        {
          "library_frame": true,
          "exclude_from_grouping": false,
          "filename": "proc.go",
          "abs_path": "/home/andrew/tools/go/1.13.0/src/runtime/proc.go",
          "line": {
            "number": 203
          },
          "function": "main",
          "module": "runtime"
        },
        {
          "library_frame": true,
          "exclude_from_grouping": false,
          "filename": "asm_amd64.s",
          "abs_path": "/home/andrew/tools/go/1.13.0/src/runtime/asm_amd64.s",
          "line": {
            "number": 1357
          },
          "module": "runtime",
          "function": "goexit"
        }
      ],
      "handled": true,
      "module": "net",
      "attributes": {
        "op": "dial",
        "temporary": true,
        "net": "tcp",
        "timeout": true
      },
      "type": "OpError",
      "message": "dial tcp: i/o timeout"
    },
    {
      "module": "internal/poll",
      "handled": true,
      "attributes": {
        "temporary": true,
        "timeout": true
      },
      "type": "TimeoutError",
      "message": "i/o timeout"
    }
  ],
  "culprit": "main",
  "id": "6f2355b4a2a17b654f79524b0c4436ac",
  "grouping_key": "94a9ee6562b77f4a1674faf628ce3ce0"
}

[watson]

If my memory serves me well from my Ruby days, it was possible to add properties to error objects. Those I would consider as candidates for attributes. But I'm not sure how common it is for an error in Ruby to have properties other than the error message and stack trace?