Skip to content

[DOCS] Document time_series_metric mapping parameter#78013

Merged
jrodewig merged 13 commits intoelastic:masterfrom
jrodewig:docs__time_series_metric
Sep 23, 2021
Merged

[DOCS] Document time_series_metric mapping parameter#78013
jrodewig merged 13 commits intoelastic:masterfrom
jrodewig:docs__time_series_metric

Conversation

@jrodewig
Copy link
Copy Markdown
Contributor

@jrodewig jrodewig commented Sep 20, 2021 •
edited
Loading

Documents the time_series_metric mapping parameter added with PR #76766.

Also renames the dimension parameter to time_series_dimension for PR #78012.

Previews

@jrodewig
[DOCS] Document time_series_metric mapping parameter
6cea9c7 
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
Copy Markdown
Collaborator

elasticmachine commented Sep 20, 2021

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

@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Sep 20, 2021

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
Copy Markdown
Collaborator

elasticmachine commented Sep 20, 2021

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

@jrodewig
Copy link
Copy Markdown
Contributor Author

jrodewig commented Sep 20, 2021

@elasticmachine update branch

csoulios
csoulios reviewed Sep 21, 2021
View reviewed changes
Copy link
Copy Markdown
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.

docs/reference/mapping/types/numeric.asciidoc Outdated
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
Copy Markdown
Contributor

@csoulios csoulios Sep 21, 2021 •
edited
Loading

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
Copy Markdown
Contributor Author

@jrodewig jrodewig Sep 21, 2021

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
Copy Markdown
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.

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

@jrodewig
Copy link
Copy Markdown
Contributor Author

jrodewig commented Sep 21, 2021 •
edited
Loading

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
csoulios
csoulios reviewed Sep 22, 2021
View reviewed changes
Copy link
Copy Markdown
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.

@csoulios
Copy link
Copy Markdown
Contributor

csoulios commented Sep 22, 2021

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
Copy Markdown
Contributor Author

jrodewig commented Sep 22, 2021 •
edited
Loading

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
csoulios
csoulios approved these changes Sep 23, 2021
View reviewed changes
Copy link
Copy Markdown
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 jrodewig mentioned this pull request Sep 23, 2021
Merged
jrodewig added a commit that referenced this pull request Sep 23, 2021
@jrodewig
[DOCS] Document time_series_metric mapping parameter (#78013) (#78247)
79ec031 
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

Reviewers

1 more reviewer

@csoulios csoulios csoulios approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jrodewig @elasticmachine @csoulios @jakelandis @elasticsearchmachine