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] Document time_series_metric mapping parameter #78013

Merged
merged 13 commits into from
Sep 23, 2021
Merged

[DOCS] Document time_series_metric mapping parameter #78013

merged 13 commits into from
Sep 23, 2021

Conversation

Documents the `time_series_metric` mapping parameter added with PR #76766.
@jrodewig jrodewig added :StorageEngine/TSDB You know, for Metrics >docs General docs changes v7.16.0 labels Sep 20, 2021
@jrodewig jrodewig marked this pull request as ready for review September 20, 2021 15:36
@elasticmachine elasticmachine added Team:Docs Meta label for docs team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Sep 20, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@jrodewig jrodewig requested a review from csoulios September 20, 2021 15:36
@jrodewig jrodewig added :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data :Search Foundations/Mapping Index mappings, including merging and defining field types labels Sep 20, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Sep 20, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@jrodewig
Copy link
Contributor Author

@elasticmachine update branch

Copy link
Contributor

@csoulios csoulios left a comment

Choose a reason for hiding this comment

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

I left one comment for numeric fields.

Also, in PR #78012 I renamed dimension parameter to time_series_parameter. Not sure if you want to add this change in the same or a separate PR.

For numeric fields, this parameter accepts `gauge` and `counter`. You can't
update this parameter for existing fields.
+
`scaled_float` and `unsigned_long` fields don't support this parameter.
Copy link
Contributor

@csoulios csoulios Sep 21, 2021

Choose a reason for hiding this comment

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

I am not sure where the constraint that scaled_float and unsigned_long fields don't support this parameter comes from. There is no such constraint in the code. Am I missing something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

During my testing, I wasn't able to create a scaled_float or unsigned_long field with the parameter. I assumed that was intended, but it may be a bug.

I opened #78100 for the bug report. I'll remove this sentence from the docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for opening the issue. I will check that asap.

@jrodewig
Copy link
Contributor Author

jrodewig commented Sep 21, 2021

Thanks @csoulios. I made some additional changes related to PR #78012.

This is ready for another look at your convenience.

@jrodewig jrodewig requested a review from csoulios September 21, 2021 15:01
Copy link
Contributor

@csoulios csoulios left a comment

Choose a reason for hiding this comment

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

Changes look good. I left some more comments.

docs/reference/mapping/types/numeric.asciidoc Outdated Show resolved Hide resolved
docs/reference/mapping/types/numeric.asciidoc Show resolved Hide resolved
@csoulios
Copy link
Contributor

One last thing we should add is that a numeric field cannot be both a dimension and a metric.
I have added this constraint in #78204

@jrodewig
Copy link
Contributor Author

jrodewig commented Sep 22, 2021

Thanks @csoulios. I documented the constraint with 8354931. I also noted that unsigned_long fields can be a time series dimension.

Edit: Also pushed 2d24b30 for doc values for numeric metrics.

@jrodewig jrodewig requested a review from csoulios September 22, 2021 20:27
Copy link
Contributor

@csoulios csoulios left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for iterating so fast on this!

@jrodewig jrodewig merged commit ce4b95e into elastic:master Sep 23, 2021
@jrodewig jrodewig deleted the docs__time_series_metric branch September 23, 2021 12:54
jrodewig added a commit that referenced this pull request Sep 23, 2021
Changes:
* Documents the `time_series_metric` mapping parameter for PR #76766.
* Renames the `dimension` parameter to `time_series_dimension` for PR #78012.
* Adds support for `unsigned_long` to `time_series_dimension` for PR #78204.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Search Foundations/Mapping Index mappings, including merging and defining field types :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data :StorageEngine/TSDB You know, for Metrics Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Docs Meta label for docs team Team:Search Meta label for search team v7.16.0 v8.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants