feat: API for writing metrics to InfluxDB file#23
Conversation
… README and add tests
WalkthroughA new file-based metrics provider has been introduced to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FileMeterProvider
participant File
User->>FileMeterProvider: Create with file path
User->>FileMeterProvider: Register globally
User->>FileMeterProvider: Create metric and record value
FileMeterProvider->>File: Write metric value in InfluxDB line protocol
User->>File: Read and verify metric data
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/influxive/src/test.rs (2)
29-29: Consider making the sleep duration configurable or use polling.The hard-coded 200ms sleep might be insufficient in slower CI environments or cause unnecessary delays in fast environments.
Consider using a polling approach instead:
- // Wait for the metric to be written - tokio::time::sleep(std::time::Duration::from_millis(200)).await; + // Wait for the metric to be written with polling + let mut attempts = 0; + while attempts < 20 && !test_path.exists() { + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + attempts += 1; + }
37-39: Strengthen the assertion for line protocol format validation.The current parsing assumes a specific space-separated format but could be more robust in validating the InfluxDB line protocol structure.
Consider a more comprehensive validation:
- let split = line.split(' ').collect::<Vec<&str>>(); - assert_eq!(split[0], "my.metric"); - assert!(split[1].contains(&"3.14")); + // Validate InfluxDB line protocol format: measurement[,tags] fields [timestamp] + assert!(line.starts_with("my.metric")); + assert!(line.contains("value=3.14")); + // Could also validate timestamp presence if neededcrates/influxive/src/lib.rs (1)
142-143: Consider documenting the parameter usage more explicitly.While the comment explains that host/bucket/token are not needed, it might be helpful to document this behavior more explicitly in the function's doc comment.
Consider enhancing the documentation:
/// Create an opentelemetry_api MeterProvider ready to provide metrics -/// to a file on disk. +/// to a file on disk. Note that host, bucket, and token authentication +/// parameters are not required for file-based writers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/influxive/README.md(1 hunks)crates/influxive/src/lib.rs(2 hunks)crates/influxive/src/test.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
crates/influxive/src/test.rs (3)
crates/influxive/src/lib.rs (1)
influxive_file_meter_provider(138-145)crates/influxive-writer/src/lib.rs (1)
create_with_influx_file(284-290)crates/influxive-writer/src/test.rs (1)
reader(218-218)
🔇 Additional comments (5)
crates/influxive/README.md (1)
62-85: LGTM! Well-structured documentation example.The new example section follows the established pattern of existing examples and clearly demonstrates the usage of the new file-based meter provider functionality. The code is consistent with the API design and includes a helpful note about reading the resulting data.
crates/influxive/src/test.rs (1)
4-40: Good test coverage for the new functionality.The test effectively validates the core functionality of the file meter provider by creating a metric, recording a value, and verifying the output file contents. The use of temporary directories ensures proper cleanup.
crates/influxive/src/lib.rs (3)
62-89: LGTM! Comprehensive documentation example.The documentation example is well-structured, follows the established pattern, and includes proper cleanup. The example effectively demonstrates the new file-based functionality.
136-145: Clean implementation that follows established patterns.The function design is consistent with existing APIs and appropriately reuses the
InfluxiveWriter::with_token_authmethod. The approach of using empty strings for unused parameters maintains API consistency while serving the file-based use case.
147-148: Good addition of test module.The test module inclusion enables proper testing of the new functionality.
|
❌ Found 1 compliant commit and 1 non-compliant commits in 8ac3e7d...6ffdc34. Commit 6ffdc34 by @ddd-mtl is not conform to the conventional commit specification :
|
3 participants
Adds high level function
influxive_file_meter_provider()with test and example.Final PR for #17
Summary by CodeRabbit
New Features
Documentation
Tests