-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[ML] Add Telemetry for models without adaptive allocations #129161
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
[ML] Add Telemetry for models without adaptive allocations #129161
Conversation
Added min and max allocations as attributes to the telemetry for trained models with adaptive allocations enabled. Added telemetry for models with adaptive allocations disabled or never set.
|
Hi @prwhelan, I've created a changelog YAML for you. |
|
Pinging @elastic/ml-core (Team:ML) |
|
|
||
| trainedModelsCurrentAllocations += trainedModelAssignment.totalCurrentAllocations(); | ||
| if (trainedModelAssignment.getAdaptiveAllocationsSettings() == null) { | ||
| trainedModelsFixedAllocations += trainedModelAssignment.totalCurrentAllocations(); |
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.
Here and in line 518 the code is summing the number of allocations from all deployments that do not use adaptive allocations. A single deployment could have 10 allocations and we wouldn't know if the user has 10 deployments with 1 allocation or 1 deployment with 10.
I think counting the number of deployments would be more meaningful
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.
Yeah that is a good point, we can just do an easy +1 to count the deployments
| "es.ml.trained_models.deployment.fixed_allocations.current", | ||
| "Sum of current trained model allocations that do not use adaptive allocations (either enabled or disabled)", | ||
| "allocations", | ||
| () -> new LongWithAttributes(trainedModelAllocationCounts.trainedModelsFixedAllocations, isMasterMap) |
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.
Can the project type be added to the attribute map? If there are different rules for different project types it would be useful to split the data that way
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 don't think so? It looks like it comes from serverless.project_type which isn't available here. We could move this metric to serverless, or we can use ES|QL magic to pull in the project type from other metrics via the project id.
It's possible this will get automatically added when running in serverless.
Added min and max allocations as attributes to the telemetry for trained models with adaptive allocations enabled.
Added telemetry for models with adaptive allocations disabled or never set.
Verified on QA:


