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

Rename fast field precision parameter to fast_precision #3940

Closed
wants to merge 1 commit into from

Conversation

dojiong
Copy link
Contributor

@dojiong dojiong commented Oct 11, 2023

The precision of span_start_timestamp_nanos and span_end_timestamp_nanos should be nanoseconds.

Edit from @guilload:
The conclusion of this conversation was to rename the fast field precision parameter to fast_precision. We can do so in a backward-compatible manner. The upcoming PR should also update the docs and the configuration examples in config/.

@@ -99,13 +99,14 @@ doc_mapping:
output_format: unix_timestamp_nanos
indexed: false
fast: true
precision: milliseconds
precision: nanoseconds
Copy link
Member

Choose a reason for hiding this comment

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

That's the fast field precision. The stored value remains in nanoseconds. The fast field won't compress as well with a precision in nanoseconds, that's why we opted for milliseconds since we were not sure it made sense for filtering and aggregations.

Do you have a use case for fast fields with nanoseconds precision?

@fmassot, @fulmicoton, do we want to revisit this decision?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'am exporing the tantivy index from quickwit's opentelementry traces index. I found the datetime field's precision is wrong:

  1. the span_start_timestamp_nanos field in quickwit_opentelemetry::otlp::traces::Span use nanoseconds
  2. the index's field use milliseconds
  3. the field's data stored in tantivy index use microseconds

I think the precision of span_start_timestamp_nanos should be nanoseconds according to the field's name.

Or microseconds? (the data stored in index is in microseconds)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or rename precision to fast_precision indicating that is for fastfield?

Copy link
Member

@guilload guilload Oct 12, 2023

Choose a reason for hiding this comment

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

  1. the field's data stored in tantivy index use microseconds

We should be on a tantivy version that uses nanoseconds for storing datetimes (quickwit-oss/tantivy#2016)

Or rename precision to fast_precision indicating that is for fastfield?

I totally agree.

@guilload guilload changed the title fix: timestamp precision in opentelemetry-traces index Rename fast field precision parameter to fast_precision Oct 12, 2023
@guilload
Copy link
Member

guilload commented Oct 12, 2023

In the future, I want the OTel indexes to be "templatable" to let users customize their settings.

@guilload guilload closed this Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants