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

chore(slo): Make APM indicator's index required #153311

Merged
merged 3 commits into from
Mar 20, 2023

Conversation

kdelemme
Copy link
Contributor

@kdelemme kdelemme commented Mar 20, 2023

Summary

This PR introduces a breaking change by making the APM indicator's index required when creating an SLO. Reasoning behind it is that the apm metrics index can be different from cluster to cluster, and we are already always providing the value from the UI, so it makes sense to require it from the API standpoint.

🥼 Manual testing

You can make an API call with and without the indicator.params.index field:

curl --request POST \
  --url http://localhost:5601/kibana/api/observability/slos \
  --header 'Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==' \
  --header 'Content-Type: application/json' \
  --header 'kbn-xsrf: oui' \
  --data '{
	"name": "My SLO Service Latency",
	"description": "My SLO Desc",
	"indicator": {
		"type": "sli.apm.transactionDuration",
		"params": {
			"environment": "development",
			"service": "o11y-app",
			"transactionType": "request",
			"transactionName": "GET /slow",
			"threshold": 5000,
			"filter": "",
			"index": "metrics-apm*"
		}
	},
	"timeWindow": {
		"duration": "7d",
		"isRolling": true
	},
	"budgetingMethod": "timeslices",
	"objective": {
		"target": 0.95,
		"timesliceTarget": 0.80,
		"timesliceWindow": "1m"
	}
}'

@kdelemme kdelemme added release_note:skip Skip the PR/issue when compiling release notes Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.8.0 labels Mar 20, 2023
@kdelemme kdelemme self-assigned this Mar 20, 2023
@kdelemme kdelemme marked this pull request as ready for review March 20, 2023 15:55
@kdelemme kdelemme requested a review from a team as a code owner March 20, 2023 15:55
@elasticmachine
Copy link
Contributor

Pinging @elastic/actionable-observability (Team: Actionable Observability)

Copy link
Contributor

@CoenWarmer CoenWarmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kdelemme kdelemme enabled auto-merge (squash) March 20, 2023 17:15
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 433 436 +3

Total ESLint disabled count

id before after diff
securitySolution 513 516 +3

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @kdelemme

@kdelemme kdelemme merged commit 54c4d8c into elastic:main Mar 20, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Mar 20, 2023
@kdelemme kdelemme deleted the slo/make-apm-index-required branch March 20, 2023 18:38
v1v added a commit to v1v/kibana that referenced this pull request Mar 20, 2023
…loy-my-kibana-oblt

* upstream/main: (727 commits)
  Upgrade caniuse-lite db (elastic#153318)
  [Security Solution] expanded flyout - right section - json tab implementation (elastic#152935)
  chore(slo): Make APM indicator's index required (elastic#153311)
  skip failing test suite (elastic#136688)
  [Security Solution] Fix security-solution storybook package codeowners (elastic#153307)
  [EUI] Add `scrollLock` workaround CSS to Kibana's `body` (elastic#153227)
  [Cloud Security] Show coming soon deployments of vulnerability management (elastic#153249)
  [Cloud Security] fixed onboarding link directs to cspm integration (elastic#153268)
  [Response Ops][Alerting] Reusable functions for FAAD resource installation (elastic#152849)
  remove geohash_grid aggregation support (elastic#152952)
  [Tech Debt] Reorder Rules page (elastic#152897)
  [Saved Object Finder] Add help text & left button (elastic#152742)
  [Transform] Replace SavedObjectsFinder component (elastic#153128)
  Make pipeline creation endpoint accept a full pipeline definition (elastic#153133)
  [Fleet] Displaying policy changes in Agent activity (elastic#153237)
  skip flaky suite (elastic#152852)
  [Security Solution][Endpoint] Add tests to cover RBAC entries in the Role Kibana Privileges flyout (elastic#153068)
  [Security Solution][Endpoint] Additional tests for Response Console History Log page (covers TestRail manual tests) (elastic#153042)
  [Monitoring] Display node roles in Nodes table (elastic#152127)
  Rename getEditAlertFlyout to getEditRuleFlyout (elastic#153243)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants