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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

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 12515237873

Details

  • 0 of 876 (0.0%) changed or added relevant lines in 9 files are covered.
  • 7 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.5%) to 11.308%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/cli.rs 0 3 0.0%
src/handlers/http/modal/server.rs 0 3 0.0%
src/kafka.rs 0 3 0.0%
src/handlers/http/modal/utils/ingest_utils.rs 0 19 0.0%
src/handlers/http/ingest.rs 0 39 0.0%
src/otel/logs.rs 0 106 0.0%
src/otel/otel_utils.rs 0 113 0.0%
src/otel/traces.rs 0 214 0.0%
src/otel/metrics.rs 0 376 0.0%
Files with Coverage Reduction New Missed Lines %
src/handlers/http/ingest.rs 1 66.17%
src/handlers/http/modal/utils/ingest_utils.rs 2 8.17%
src/kafka.rs 4 0.0%
Totals Coverage Status
Change from base Build 12511730436: -0.5%
Covered Lines: 2152
Relevant Lines: 19030

💛 - 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 and others added 11 commits January 2, 2025 01:52
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 b1a3c94 to 08b95c4 Compare January 2, 2025 06:55
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