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

[DOCS] Execution summary is missing in the find rules API response #1759

Closed
xcrzx opened this issue Mar 23, 2022 · 11 comments · Fixed by #1900
Closed

[DOCS] Execution summary is missing in the find rules API response #1759

xcrzx opened this issue Mar 23, 2022 · 11 comments · Fixed by #1900

Comments

@xcrzx
Copy link
Contributor

xcrzx commented Mar 23, 2022

Description

Find rules API and Get rule API response schemas do not include the execution_summary field, which was added in 8.1. We could add the following JSON to the examples in the docs to make them more complete.

{
   ...,
   "execution_summary": {
      "last_execution": {
         "date": "2022-03-23T16:06:12.787Z",
         "status": "partial failure",
         "status_order": 20,
         "message": "This rule is attempting to query data from Elasticsearch indices listed in the \"Index pattern\" section of the rule definition, however no index matching",
         "metrics": {
            "total_search_duration_ms": 135,
            "total_indexing_duration_ms": 15,
            "execution_gap_duration_s": 0,
         }
      }
   }
@jmikell821
Copy link
Contributor

@xcrzx is this executive summary different from this description: A JSON object containing a summary and the returned rules?

@xcrzx
Copy link
Contributor Author

xcrzx commented Mar 24, 2022

Hey @jmikell821, execution_summary is included for every active rule in the response and contains information about the last rule execution (like status, warnings, etc.). So yeah, it's a bit different from the top-level summary, which contains pagination/total rules found.

@jmikell821 jmikell821 self-assigned this Mar 24, 2022
@banderror
Copy link
Contributor

banderror commented Mar 24, 2022

@xcrzx @jmikell821 I think we shouldn't document execution_summary yet.

Rule execution data like statuses and metrics has never been documented before, although our endpoints have been returning them in the responses (in a different format). We introduced a breaking change to this part of our responses in one of the prior releases, and there is a chance we will need to do this again. There are some current plans to consolidate Alerting Framework rule statuses and Security Solution's statuses, as well as Framework and solution-specific execution metrics. This could be a large enough change to break something. You can check the internal RFC and elastic/kibana#112193 for more details.

Let's keep this part undocumented until the work described in the above-mentioned RFC and ticket is complete, and we have a clear understanding of how we're going to be able to extend this data structure in the future (add new metrics etc). This will give us a chance to introduce a breaking change if we absolutely need to.

@jmikell821
Copy link
Contributor

@banderror thanks for the update! I will hold off on this for now; please keep me posted when it's appropriate to pick this back up again. Thanks!

@banderror
Copy link
Contributor

We will keep you posted @jmikell821, thank you for your understanding! This ticket will be in our backlog, we won't forget about it. Hope that in the next 2-3 releases we will have full confidence in our rule monitoring data structures and behavior, and will be aligned with Response Ops on the APIs and how they are going to evolve over time. Thank you 🙏

@xcrzx
Copy link
Contributor Author

xcrzx commented Mar 25, 2022

@banderror, I would strongly advise against introducing breaking changes to Get Rules and Find Rules APIs if that's possible. We've never documented the execution_summary part of the response, but other than that, these APIs are well documented and highly adopted. So I would expect many users to utilize them.

I created this issue after @cavokz had reached out to me asking for a substitution to the rues/_find_statuses API removed in 8.0. And I don't see any good alternative to retrieve rule execution status, last errors, etc., other than reading this info from rule's execution_summary. So we already have users who rely on the current response schema, and any changes could be breaking for them.

Given all the above and considering company-wide Make it Minor initiative, I think the best option would be to document the missing part of the response and limit possible breaking changes.

That's said, I'm open to other opinions. @peluja1012, what do you think about that?

@banderror
Copy link
Contributor

@jmikell821 we had a chat on this and agreed that we could document execution_summary in 8.2 if there's a way of marking this part of the response as "experimental" or "beta" or something like that. Do you know if there was a precedent of marking any part of an API like that in the Kibana or Security Solution docs?

@banderror
Copy link
Contributor

@xcrzx We return execution_summary in responses of some of the other rule management API endpoints, so we will need to remember to update the docs for them as well.

CRUD endpoints:

Bulk CRUD endpoints (even if we're going to deprecate them):

Search endpoints:

Could you please double-check in case I missed something?

@jmikell821
Copy link
Contributor

Hey @xcrzx and @banderror I want to be clear on what's expected here for 8.2. So we are including the execution summary, but are marking it as beta, correct? Can one of you open a PR with the changes?

@banderror - we can add the beta admonition to those sections with this syntax:

beta::[]

For the deprecated endpoints, we are just adding a note next to each bulk action that contains the deprecated endpoints?

@banderror
Copy link
Contributor

Hey @jmikell821, @xcrzx is off this week. I'll try to find time to work on this, otherwise, I'll move this ticket to 8.3.

Thank you for the info regarding https://github.com/elastic/docs#beta-dev-and-preview-experimental, this is great! Is there a guideline on choosing the right label: beta vs dev vs preview?

@banderror banderror self-assigned this Apr 19, 2022
@jmikell821
Copy link
Contributor

Hey @jmikell821, @xcrzx is off this week. I'll try to find time to work on this, otherwise, I'll move this ticket to 8.3.

Thank you for the info regarding https://github.com/elastic/docs#beta-dev-and-preview-experimental, this is great! Is there a guideline on choosing the right label: beta vs dev vs preview?

Good question @banderror. Here's some feedback:

These features differ based on expectations around longevity. A beta feature is one that we expect will be around for the long term in substantially the same form as it is currently. There may be bugs, but we are sure we want to have that feature in the product. A technical preview (previously experimental feature) is subject to more change. The interfaces may change wildly, and we may even decide that the feature should not have been included in the product originally.

That being said, I think beta probably applies best here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants