Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APM] Annotation for time-series charts on service.version #51426

Closed
5 tasks
formgeist opened this issue Nov 22, 2019 · 12 comments · Fixed by #52640
Closed
5 tasks

[APM] Annotation for time-series charts on service.version #51426

formgeist opened this issue Nov 22, 2019 · 12 comments · Fixed by #52640
Assignees
Labels
roadmap Team:APM - DEPRECATED Use Team:obs-ux-infra_services. v7.6.0

Comments

@formgeist
Copy link
Contributor

formgeist commented Nov 22, 2019

Summary

Related issues:

Design issue
Show service.version filter in UI filters


Figma prototype

We wish to annotate each different version value found in service.version for the selected time range in all the time-series charts in APM. The annotation is represented as a vertical accented line with an icon and tooltip that contains additional information such as value i.e. 1.2.0 and number of associated instances of the service where this specific version is found.

Tasks

  • Display vertical annotation with EuiIcon tag in the EuiColorSecondary for each annotation instance on all time-series charts on the Transactions overview, detail and Metrics detail pages.
  • Display tooltip upon hover positioned at the top
    • @timestamp (tooltip title)
    • service.version value
  • Show service.version annotation legend by the other legends (appended the chart values) with the option to hide/show the annotation per chart. This is not persistent.

Screen examples

Kapture 2019-11-27 at 12 07 08

01 P1 - Service version annotations - Transactions (popovers and tooltips examples)

03 P1 - Service version annotations - Metrics detail

Example query for service.version

To query the metrics, error and transaction indices to get all the versions that were first seen in the given time frame (replace START and END with start end of the date range):

{
  "query": {
    "bool": {
      "filter": [
        {
          "terms": {
            "processor.event": [ "metric", "error", "transaction" ]
          }
        },
        {
          "term": {
            "service.name": "opbeans-node"
          }
        },
        {
          "exists": {
            "field": "service.version"
          }
        }
      ]
    }
  },
  "size": 0, 
  "aggs": {
    "by_version": {
      "terms": {
        "field": "service.version"
      },
      "aggs": {
        "first_seen": {
          "min": {
            "field": "@timestamp"
          }
        },
        "select": {
          "bucket_selector": {
            "buckets_path": {
              "first_seen": "first_seen.value"
            },
            "script": "params.first_seen >= __START__ && params.first_seen <= __END__"
          }
        }
      }
    }
  }
}
@formgeist formgeist added Team:APM - DEPRECATED Use Team:obs-ux-infra_services. v7.6.0 labels Nov 22, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@dgieselaar
Copy link
Member

I'm still not sure what instances would mean. When we first see any processor event with a new version, it's always going to be one instance. I don't expect this version to be applied to all instances at once, and have a clear cutoff point where all new processor events from all instances have the new version.

@formgeist
Copy link
Contributor Author

@nehaduggal For the above reasons, do you think it's worth deferring the instance count in this first version? Or do you have other thoughts about showing the instance count in the tooltip per version annotation?

@nehaduggal
Copy link

We can drop the instance count for now.

@dgieselaar although we can't identify instance count, we should be able to detect if multiple versions are running for a particular service(based on the service version that is reporting up), right? We will have to handle that case where we start seeing multiple versions of a service reporting into the same APM Service. If we can capture multiple versions, maybe for the first iteration we can drop the instance count and replace it with number of versions detected?

@sorenlouv
Copy link
Member

sorenlouv commented Nov 25, 2019

maybe for the first iteration we can drop the instance count

I don't think this is a matter of dropping it for now, and doing it later. I think we first need to define exactly what we want.

Like @dgieselaar says above, if we detect a deployment at 13:37 on November 1st there'll be exactly one instance with this particular version at this time. A different approach might be:

  1. show the number of instances with the particular version within 1 hour after the first
  2. or, show the number of instances which have been observed with that version up until now

I think option 2 might be the most useful one. But the challenge is that if we show an annotation like:

**Nov 1st, 13:37** 

Version: 2.1
instances: 193 

Then people might think that 193 instances had version 2.1 at Nov 1st, 13:37. When in reality only 1 instance had the version at that time, and the rest got it later (somewhere between then and now).

We could make this more explicit with some added copy:

**First observed on Nov 1st, 13:37** 

Version: 2.1
instances: 193 (deployed until now)

Again, this is not bulletproof because a new version, 2.2, might have been released and some of those instances have since deployed that version. So there are no guarantees that 193 instances have simultaneously been running version 2.1. LMKWYT.

@dgieselaar
Copy link
Member

Alternatively we could also show a semi-transparent overlay between the first_seen of the new version and the last_seen of the old version. Couple of edge cases though: rollbacks, overlapping deploys, and very old instances that keep reporting data from older deployments (which will definitely be happening with RUM).

@formgeist
Copy link
Contributor Author

@sqren I completely agree with you that we need to have a clear objective with showing that count, and that question no. 2 is the one I've had in my mind would be the best. Also agree that some more explicit copy will help the user understand that value, if that's what we decide.

@dgieselaar If I understand you correctly, I'd be concerned with adding this to all time-series charts. Might be better to save it for a dedicated versions chart?

@dgieselaar
Copy link
Member

@formgeist Yeah, agreed 👍

@formgeist
Copy link
Contributor Author

Design update

I've updated the design to reflect some smaller clarifications that we came to agreement on in a Zoom yesterday; we are not going to show instances count for this first iteration and I've updated the color of the annotation to not be "red", but use our secondary "green" to indicate new version.

@cauemarcondes
Copy link
Contributor

@formgeist do you want to show the tooltip only when the user hovers over the service version icon or the whole line?

@formgeist
Copy link
Contributor Author

formgeist commented Dec 3, 2019

I originally had it show up when you hover the whole chart, but I think it will collide with the chart tooltip, so better implement it on hover on the service.version icon. Thanks 👍

dgieselaar added a commit to dgieselaar/kibana that referenced this issue Dec 18, 2019
@formgeist
Copy link
Contributor Author

I've opened a separate issue for enhancing the styles. We can go ahead and merge this implementation and simply enhance it with the new style in another. #53483

dgieselaar added a commit that referenced this issue Dec 18, 2019
* [APM] Add version annotations to timeseries charts

Closes #51426.

* Don't subdue 'Version' text in tooltip

* Optimize version queries

* Don't pass radius/color to indicator
dgieselaar added a commit to dgieselaar/kibana that referenced this issue Dec 18, 2019
* [APM] Add version annotations to timeseries charts

Closes elastic#51426.

* Don't subdue 'Version' text in tooltip

* Optimize version queries

* Don't pass radius/color to indicator
dgieselaar added a commit that referenced this issue Dec 24, 2019
* [APM] Add version annotations to timeseries charts

Closes #51426.

* Don't subdue 'Version' text in tooltip

* Optimize version queries

* Don't pass radius/color to indicator

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
roadmap Team:APM - DEPRECATED Use Team:obs-ux-infra_services. v7.6.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants