newrelic / nr1-slo-r

NR1 SLO-R allows you to define, calculate and report on service-level objective (SLO) attainment.
https://discuss.newrelic.com/t/track-your-service-level-objectives-with-the-slo-r-nerdpack/90046
Apache License 2.0
21 stars 21 forks source link

Defining SLO with Latency (Calculated) indicator through Entity Explorer brings up interface for alert-based Latency SLOs #132

Open ghost opened 3 years ago

ghost commented 3 years ago

Description

In SLO/R, one can define a SLO using the 'Latency (Calculated)' indicator, which uses transaction data, or with the 'Latency' indicator, which uses alerts. When selecting 'Latency (Calculated)' in the SLO/R Nerdpack, the system behaves as expected, bringing up a transaction picker. When selecting the same in entity explorer, the system brings up an alert picker instead of the transaction picker.

Steps to Reproduce

  1. Define SLO in Entity Explorer view
  2. Select indicator 'Latency (Calculated)'
  3. Input fields for 'Latency' appear.

Expected Behaviour

Selecting 'Latency (Calculated)' in Entity Explorer should bring up the Duration Limit and Transactions fields, not Alerts.

Relevant Logs / Console output

Your Environment

Additional context

ghost commented 3 years ago

This is probably a miss from my previous PR adding the 'Latency (Calculated)' indicator, I will take ownership of investigating and fixing this.

ghost commented 3 years ago

The changes to slo-r-main to add Calculated Latency were not made in slo-r-entity.

@ricegi @kpeet It looks like both main and entity components are providing similar functionality with only minor differences in implementation. Is there a compelling reason to maintain them separately, or should we consider merging them into one shared component?

khpeet commented 3 years ago

@andrewseling - I fixed this in my latest fork - It does make sense to make the form a shared component - but I will leave that to @ricegi 's discretion since there are some differences in the forms (i.e - SLO Groups)

khpeet commented 3 years ago

this is fixed with https://github.com/newrelic/nr1-slo-r/pull/133 - should be good to close.