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

When defining multiple SLOs through Entity Explorer view, input validation does not handle tags correctly. #131

Open ghost opened 4 years ago

ghost commented 4 years ago

Description

After defining a SLO in entity explorer view, defining an additional SLO will bring up the form with tags persisted from the previous SLO. After completing the rest of the form, clicking "add new service" will throw a validation error. It appears that the validator is looking for a tag or a group to have been added as part of the definition operation, and is not recognizing the persisted tags. This causes the validator to think that there are no tags or groups defined, which is invalid.

Removing a tag and re-adding it will cause the validator to successfully create the SLO with all tags, new or persisted from the previous SLO.

Steps to Reproduce

  1. Define a SLO in Entity Explorer with tags
  2. Click "define a SLO" and define a second SLO. Tags will be persisted from the previous SLO. Click "Add new service."
  3. Receive error

To resolve, acknowledge the error, remove a tag, and re-add it. SLO/R now recognizes that the tag exists, and allows the new service to be added. All the persisted tags will also be added, suggesting that the issue is not with the SLO definition itself, but with whatever component recognizes that a tag or group has been added (formik?).

Expected Behaviour

Persisted tags should be treated like newly added tags rather than causing a validation error when adding a new service.

Relevant Logs / Console output

Problem with SLO definition! Please validate you have an SLO Name, Group, and Target defined. Also ensure your Error Budget includes at least one transaction and one defect, or your Alert driven SLO includes an Alert.

Your Environment

Additional context

khpeet commented 4 years ago

@andrewseling - This should be fixed in my latest fork