-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Alerting] Adding telemetry usage counter for create rule API when predefined ID is specified #108572
[Alerting] Adding telemetry usage counter for create rule API when predefined ID is specified #108572
Conversation
@@ -274,7 +277,12 @@ export class AlertingPlugin { | |||
// Routes | |||
const router = core.http.createRouter<AlertingRequestHandlerContext>(); | |||
// Register routes | |||
defineRoutes(router, this.licenseState, plugins.encryptedSavedObjects); | |||
defineRoutes({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this interface to options object instead of list of params bc the list is getting long and I'm planning to add logger as well in another PR
…ing/telemetry-for-predefined-rule-ids
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
…ing/telemetry-for-predefined-rule-ids
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created two rules, one in the default space and one in a custom space.
POST .kibana_8*/_search
{
"_source": ["_id"],
"query": {
"term": {
"type": {
"value": "alert"
}
}
}
}
#! this request accesses system indices: [.kibana_8.0.0_001], but in a future major version, direct access to system indices will be prevented by default
{
"took" : 1,
"timed_out" : false,
"_shards" : {
"total" : 1,
"successful" : 1,
"skipped" : 0,
"failed" : 0
},
"hits" : {
"total" : {
"value" : 2,
"relation" : "eq"
},
"max_score" : 1.8352444,
"hits" : [
{
"_index" : ".kibana_8.0.0_001",
"_id" : "chrisspace:alert:5e7a4482-c161-46a7-830c-9e91eddf904c",
"_score" : 1.8352444,
"_source" : { }
},
{
"_index" : ".kibana_8.0.0_001",
"_id" : "alert:5e7a4482-c161-46a7-830c-9e91eddf904c",
"_score" : 1.8352444,
"_source" : { }
}
]
}
}
However, the telemetry says:
{
"usage_counters": {
"dailyEvents": [
{
"domainId": "alerts",
"counterName": "ruleCreatedWithPredefinedIdInDefaultSpace",
"counterType": "count",
"lastUpdatedAt": "2021-08-16T13:55:22.332Z",
"fromTimestamp": "2021-08-16T00:00:00Z",
"total": 2
},
{
"domainId": "alerts",
"counterName": "ruleCreatedWithPredefinedIdInCustomSpace",
"counterType": "count",
"lastUpdatedAt": "2021-08-16T13:56:12.337Z",
"fromTimestamp": "2021-08-16T00:00:00Z",
"total": 1
}
]
}
}
(Note the total: 2
in the default space)
Is that expected somehow?
@chrisronline Hmm...is there any chance you tried to create the default space rule twice? You would've gotten a version conflict message the second time (and no rule would have been created) but the usage counter would have ticked up. This PR is counting any attempt to use the route, not just the successful rule creation attempts. |
@ymao1 I tried it the third time and I'm only see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Nice work!
…ing/telemetry-for-predefined-rule-ids
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM! I tested locally and was able to see the counters go up accordingly 👍 couple of nits below.
spaceId === undefined || spaceId === 'default' | ||
? 'ruleCreatedWithPredefinedIdInDefaultSpace' | ||
: 'ruleCreatedWithPredefinedIdInCustomSpace'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I wonder if we can build visualizations on the sum of both fields 🤔 If not, should we track ruleCreatedWithPredefinedIdInCustomSpace
and ruleCreatedWithPredefinedId
instead? (one is to track custom IDs in non-default space, and the other is any space).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea! I'm currently trying to figure out how to get access to the telemetry cluster to see how these daily events are visualized, but given that it's an array of objects, it's likely that it might not be straightforward to do a sum. Since I'd like to get this in before FF, I updated in this commit with your suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but given that it's an array of objects, it's likely that it might not be straightforward to do a sum.
Oh, interesting 🤔 at least we'll start by tracking and then we can figure out visualizing if ever it's a problem.
I'll reach out with the telemetry cluster info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing to note is the telemetry team is able to perform transformations on the data in the "middle layer" of the process (which is basically a cloud service that sits in the middle of Kibana sending the data, and it being indexed into the ES cluster) so it's possible to fix any issues outside of our deployment model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's really good to know! Thanks, I'll add it to my list of questions (w/ the increasing field count each release).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is interesting! Are those transformations documented anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find anything public atm, but I'd ping @mindbat for more information as he knows all about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymao1 @mikecote The transformations are part of the new telemetry system we've been building out 😀
We did a presentation at GAH about the capabilities, and we've got docs on how to build/use a custom index to take advantage of them here. The goal is to make things as self-service as possible, so folks like yourselves can have full control over how the data gets indexed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mindbat Thank you for the links! We will review them and look into how we can take advantage of this system!
x-pack/plugins/alerting/server/routes/lib/count_usage_of_predefined_ids.ts
Outdated
Show resolved
Hide resolved
…ing/telemetry-for-predefined-rule-ids
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/lens/drag_and_drop·ts.lens app lens drag and drop tests keyboard drag and drop should drop a field to workspaceStandard Out
Stack Trace
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @ymao1 |
…edefined ID is specified (elastic#108572) * Adding telemetry usage counter for create rule API when predefined ID is specified * Checking explicitly for default space id * Handling undefined space id when spaces is disabled * Consolidating logic in single function * Tracking total usage and custom usage
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…edefined ID is specified (#108572) (#108812) * Adding telemetry usage counter for create rule API when predefined ID is specified * Checking explicitly for default space id * Handling undefined space id when spaces is disabled * Consolidating logic in single function * Tracking total usage and custom usage Co-authored-by: ymao1 <[email protected]>
Resolves #108417
Summary
Adds a telemetry usage counter to track the number of a times rules are created with pre-defined IDs via the API. Differentiates between default and custom space.
To Verify
Checklist
Delete any items that are not applicable to this PR.