search

elastic / package-spec

EPR package specifications
Other
17 stars 70 forks source link

[Change Proposal] Add support for counted_keyword field type #698

Closed JonasKunz closed 1 month ago

JonasKunz commented 7 months ago

Recently the counted_keyword field type has been added to elasticsearch ((https://github.com/elastic/elasticsearch/pull/101826).

We would like to add a new field of this type to the APM package (see https://github.com/elastic/apm-server/issues/12054).

Unfortunately this is not yet possible, because the elastic-package utility complains about counted_keyword being an invalid field type:

jonas@Jonass-MBP apm % ../../elastic-package build
2024/01/23 13:48:29  WARN CommitHash is undefined, in both /Users/jonas/.elastic-package/version and the compiled binary, config may be out of date.
Build the package
README.md file rendered: /Users/jonas/git/integrations/packages/apm/docs/README.md
2024/01/23 13:48:30  INFO License text found in "/Users/jonas/git/integrations/LICENSE.txt" will be included in package
Error: building package failed: invalid content found in built zip package: found 6 validation errors:
   1. item [.DS_Store] is not allowed in folder [/Users/jonas/git/integrations/build/packages/apm-8.13.0-preview-1.zip/data_stream]
   2. item [.DS_Store] is not allowed in folder [/Users/jonas/git/integrations/build/packages/apm-8.13.0-preview-1.zip/data_stream/traces]
   3. item [.DS_Store] is not allowed in folder [/Users/jonas/git/integrations/build/packages/apm-8.13.0-preview-1.zip/data_stream/traces/elasticsearch]
   4. file "/Users/jonas/git/integrations/build/packages/apm-8.13.0-preview-1.zip/data_stream/traces/fields/fields.yml" is invalid: field 101.type: 101.type must be one of the following: "aggregate_metric_double", "alias", "histogram", "constant_keyword", "text", "match_only_text", "keyword", "long", "integer", "short", "byte", "double", "float", "half_float", "scaled_float", "date", "date_nanos", "boolean", "binary", "integer_range", "float_range", "long_range", "double_range", "date_range", "ip_range", "group", "geo_point", "object", "ip", "nested", "flattened", "wildcard", "version", "unsigned_long"

So to me it seems like we have to add the counted_keyword to the field type specification, maybe this PR can serve as a template.

### Tasks
- [x] Add field type to the package spec in a new minor version
- [ ] Add validation for subfields of this type in elastic-package
- [ ] Implement support in Kibana/Fleet, if needed
- [ ] Increase max package-spec version in Kibana configuration for serverless
jsoriano commented 7 months ago

Thanks for opening this issue.

Apart of adding the type in the package-spec, we will need to add the subfields validation to elastic-package, as was done for histograms in https://github.com/elastic/elastic-package/pull/939/. Also, the change will need to be included in a new package spec minor, probably 3.1, and this version should be used in the kibana configuration for serverless deployments that support it, this can be updated in these files: https://github.com/elastic/kibana/blob/ee11d8bbce96acfc4f1eeff5588087c360b74ab9/config/serverless.oblt.yml#L44

I have created a task list including these tasks.

AlexanderWert commented 7 months ago

Thanks @jsoriano !

Do you have an estimate when we can expect this to be implemented? Just to understand the impact / delay on a project we currently execute on.

jsoriano commented 7 months ago

Do you have an estimate when we can expect this to be implemented? Just to understand the impact / delay on a project we currently execute on.

I am not sure when we will have a slot for this, pinging @jen-huang and @kpollich about planning.

Is there a deadline when you would need this? Do you have workarounds in the meantime?

Btw, I have added another point to the tasklist about adding support in Fleet if needed.

jen-huang commented 7 months ago

@AlexanderWert I would like to understand the priority of the project(s) this blocks too. FYI we are happy to provide guidance and PR reviews if your team would like to contribute instead of being blocked by us. The list of tasks and reference PRs looks like a good place to start.

JonasKunz commented 7 months ago

I'll try to implement the necessary changes, so that our apm-package update can hopefully make it into 8.13.

kpollich commented 1 month ago

I believe this was closed by https://github.com/elastic/package-spec/pull/707. Please reopen if this is incorrect.