Skip to content

Conversation

@przemekwitek
Copy link
Contributor

@przemekwitek przemekwitek commented Apr 13, 2021

This PR adds telemetry for transform features, i.e.: the number of existing transforms using each feature.
This PR adds support for 4 features: pivot, latest, sync and retention_policy but the code is generic enough to handle other features too.

Example response from _xpack/usage:

{
  ...
  "transform": {
    "enabled": true,
    "available": true,
    "transforms": {
      "_all": 4,
      "stopped": 2,
      "started": 2
    },
    "feature_counts": {
      "pivot": 3,
      "latest": 1,
      "sync": 2,
      "retention_policy": 1,
    },
    "stats": {
      ...
    }
  }
  ...
}

Relates #68461

@przemekwitek przemekwitek force-pushed the transform_telemetry branch 7 times, most recently from 0091d5a to 8c2a685 Compare April 14, 2021 09:24
@przemekwitek przemekwitek changed the title [Transform] Add telemetry support for functions [Transform] Add telemetry support for transform features Apr 14, 2021
@przemekwitek przemekwitek removed the WIP label Apr 14, 2021
@przemekwitek przemekwitek marked this pull request as ready for review April 14, 2021 09:27
@elasticmachine elasticmachine added the Team:ML Meta label for the ML team label Apr 14, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@przemekwitek
Copy link
Contributor Author

run elasticsearch-ci/2

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

Looks pretty good.

I wonder if we can use runtime_mappings to capture the configs from before the mapping was updated? Otherwise, we effectively lose all that data :/

I think there is a way to use runtime_mappings + the index mapping to handle this..

Copy link

@hendrikmuhs hendrikmuhs left a comment

Choose a reason for hiding this comment

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

LGTM

The idea with the runtime mappings is good, but would require a lot of testing. E.g. on a mixed version cluster with old <7.11 nodes.

The other side-effect: if search.allow_expensive_queries is false it will break even on newer clusters.

I think it is not worth the potential trouble.

Note: A _update call moves the config to the latest internal index, so eventually (latest 8) configs will be stored in an index with proper mappings.

We could think of making this move easier, e.g. letting the UI suggest the update or do the update on _start if we detect an older config. Something to discuss as a follow up.

Copy link
Contributor Author

@przemekwitek przemekwitek left a comment

Choose a reason for hiding this comment

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

Thanks @benwtrent for the suggestion to use runtime mappings. I was thinking about sth along these lines but by what @hendrikmuhs explained I also found it not worth the potential trouble.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants