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

feat: Custom Flattening for OTEL logs, metrics and traces #1043

Merged

Conversation

nikhilsinhaparseable
Copy link
Contributor

  • add proto files for metrics and trace

  • add compiled rust files for metrics and trace protobuf files

  • add separate handlers for OTEL logs, metrics and traces

  • custom flattening added for OTEL logs, metrics and traces

    use endpoints

  • /v1/logs for OTEL logs

  • /v1/metrics for OTEL metrics

  • /v1/traces for OTEL traces

    add custom header X-P-Log-Source when using endpint api/v1/ingest

  • otel-logs for OTEL logs

  • otel-metrics for OTEL metrics

  • otel-traces for OTEL traces

@nitisht nitisht requested a review from de-sh December 19, 2024 11:37
@nikhilsinhaparseable nikhilsinhaparseable force-pushed the otel-metrics-traces-flattening branch from a80beb2 to 6b4b20d Compare December 25, 2024 09:39
@de-sh
Copy link
Contributor

de-sh commented Dec 25, 2024

@nikhilsinhaparseable can you document the code with expectations similar to this: https://github.com/parseablehq/parseable/pull/1024/files#diff-c45a9621970d114a8f2791b564a0a3dd8bf7825dcf549fae486a98032723c12eR41-R57

The PR is extremely hard to review, given the exploding scope of code, including generated code, I would heavily suggest we consider moving this to the build stage and require build machines to have protoc and hence not have to include the code in the repo and depend on the tonic code generator to do the job of maintaining the code for us.

Please also move files into related module e.g. otel_logs.rs ~> otel/logs.rs

As for fixing the clippy, this issue is caused by including generated code into the repo, which is a bad practice, but given our circumstance, as a short term fix, we can use this:

#[allow(clippy::all)]
pub mod proto;

@nikhilsinhaparseable nikhilsinhaparseable force-pushed the otel-metrics-traces-flattening branch from 6b4b20d to 570a7f4 Compare December 25, 2024 19:09
Cargo.toml Outdated Show resolved Hide resolved
@de-sh
Copy link
Contributor

de-sh commented Dec 26, 2024

We can consider using https://crates.io/crates/opentelemetry-proto

@coveralls
Copy link

coveralls commented Dec 26, 2024

Pull Request Test Coverage Report for Build 12621906304

Details

  • 89 of 1066 (8.35%) changed or added relevant lines in 11 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.3%) to 12.483%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/kafka.rs 0 1 0.0%
src/handlers/http/modal/server.rs 0 3 0.0%
src/utils/json/mod.rs 11 14 78.57%
src/event/format/mod.rs 2 10 20.0%
src/handlers/http/modal/utils/ingest_utils.rs 8 32 25.0%
src/handlers/http/ingest.rs 58 129 44.96%
src/otel/logs.rs 0 110 0.0%
src/otel/otel_utils.rs 0 117 0.0%
src/otel/traces.rs 0 220 0.0%
src/otel/metrics.rs 0 420 0.0%
Files with Coverage Reduction New Missed Lines %
src/kafka.rs 4 0.0%
Totals Coverage Status
Change from base Build 12621383951: -0.3%
Covered Lines: 2467
Relevant Lines: 19763

💛 - Coveralls

@nikhilsinhaparseable
Copy link
Contributor Author

@de-sh removed the pre-compiled and the protobuf files.
I am using the crate you suggested.
Please review again.

Cargo.toml Outdated Show resolved Hide resolved
@nitisht
Copy link
Member

nitisht commented Dec 29, 2024

Is this ready now for merge?

@nikhilsinhaparseable nikhilsinhaparseable force-pushed the otel-metrics-traces-flattening branch 3 times, most recently from 1ef9619 to 74ddc86 Compare January 4, 2025 02:47
@@ -52,8 +52,17 @@ impl EventFormat for Event {
static_schema_flag: Option<&String>,
time_partition: Option<&String>,
schema_version: SchemaVersion,
log_source: &str,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should move away from using str given how wide the possibilities are, please consider using enums similar to what we are doing with SchemaVersion above, it will also make things easier to read

value_json.insert(key.to_string(), Value::Bool(bool_val));
}
OtelValue::IntValue(int_val) => {
value_json.insert(key.to_string(), Value::String(int_val.to_string()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remind me why we are doing int to string, instead of number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

otel collector sends int value as string, but now it can be serialised as int, hence, i think we can now change it to number, let me test it out

@nitisht
Copy link
Member

nitisht commented Jan 5, 2025

@nikhilsinhaparseable can you resolve the conflicts

nikhilsinhaparseable and others added 11 commits January 5, 2025 12:39
add proto files for metrics and trace
add compiled rust files for metrics and trace protobuf files
add separate handlers for OTEL logs, metrics and traces
custom flattening added for OTEL logs and metrics
add custom flattening for otel traces
use endpoints
`/v1/logs` for OTEL logs
`/v1/metrics` for OTEL metrics
`/v1/traces` for OTEL traces

add custom header X-P-Log-Source when using endpint `api/v1/ingest`
`otel-logs` for OTEL logs
`otel-metrics` for OTEL metrics
`otel-traces` for OTEL traces
Co-authored-by: Devdutt Shenoi <[email protected]>
Signed-off-by: Nikhil Sinha <[email protected]>
used `opentelemetry-proto` crate
deleted compiled rust files and protobuf files
Co-authored-by: Devdutt Shenoi <[email protected]>
Signed-off-by: Nikhil Sinha <[email protected]>
@nikhilsinhaparseable nikhilsinhaparseable force-pushed the otel-metrics-traces-flattening branch from 59fac1d to 6908630 Compare January 5, 2025 18:20
@nitisht nitisht merged commit 622b9a2 into parseablehq:main Jan 6, 2025
9 checks passed
parmesant pushed a commit to parmesant/parseable that referenced this pull request Jan 13, 2025
…q#1043)

custom flattening for OTEL data

add proto files for metrics and trace
add compiled rust files for metrics and trace protobuf files
add separate handlers for OTEL logs, metrics and traces
custom flattening added for OTEL logs and metrics

custom flattening for OTEL traces

use endpoints
`/v1/logs` for OTEL logs
`/v1/metrics` for OTEL metrics
`/v1/traces` for OTEL traces

add custom header X-P-Log-Source when using endpint `api/v1/ingest`
`otel-logs` for OTEL logs
`otel-metrics` for OTEL metrics
`otel-traces` for OTEL traces

---------

Signed-off-by: Nikhil Sinha <[email protected]>
Co-authored-by: Devdutt Shenoi <[email protected]>
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.

4 participants