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

Created on 11 Jul 2018 · 26Comments · Source: elastic/kibana

_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

apm Backlog blocked good first issue

Source

formgeist

All 26 comments

Pinging @elastic/apm-ui

elasticmachine on 11 Jul 2018

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

sqren on 11 Jul 2018

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.

formgeist on 11 Jul 2018

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

sqren on 11 Jul 2018

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.

formgeist on 11 Jul 2018

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.

beniwohli on 11 Jul 2018

^^ 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.

formgeist on 11 Jul 2018

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.

sqren on 11 Jul 2018

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.

formgeist on 11 Jul 2018

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

sqren on 24 Oct 2019

👍1

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

formgeist on 25 Oct 2019

Cool, updated.

sqren on 25 Oct 2019

@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

sqren on 25 Oct 2019

@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?

formgeist on 25 Oct 2019

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

sqren on 25 Oct 2019

@sqren Sure, I'll open a PR 🤓

formgeist on 25 Oct 2019

❤1

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

formgeist on 25 Oct 2019

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?

dgieselaar on 30 Oct 2019

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.

beniwohli on 30 Oct 2019

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 :)

sqren on 30 Oct 2019

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

jalvz on 30 Oct 2019

@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?

dgieselaar on 18 Nov 2019

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

beniwohli on 18 Nov 2019

👍1

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.

sqren on 18 Nov 2019

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"
}

axw on 19 Nov 2019

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?