-
Notifications
You must be signed in to change notification settings - Fork 18
Implements logging using OpenTelemetry as per #67 #103
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
base: develop
Are you sure you want to change the base?
Conversation
Hefty commit, I will have to learn how all of this works myself before I review it. Will try and get it done before the weekend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements comprehensive OpenTelemetry logging and tracing as per issue #67. The changes introduce distributed tracing capabilities with proper instrumentation throughout the application.
Key changes:
- Added OpenTelemetry dependencies and configuration for tracing and metrics
- Instrumented key functions with
#[tracing::instrument]
attributes - Implemented graceful shutdown with proper telemetry cleanup
- Added Debug derives to structs for better tracing support
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
Cargo.toml | Added OpenTelemetry dependencies for tracing, metrics, and OTLP export |
src/main.rs | Implemented complete OpenTelemetry setup with tracer/meter providers and graceful shutdown |
src/routes.rs | Added tracing instrumentation to router setup function |
src/models/*.rs | Added Debug derives to structs for better tracing support |
src/graphql/queries/*.rs | Added tracing instrumentation and logging to GraphQL query functions |
src/graphql/mutations/attendance_mutations.rs | Added tracing instrumentation to mutation functions |
src/daily_task/mod.rs | Added tracing instrumentation to daily task functions |
Comments suppressed due to low confidence (1)
src/main.rs:1
- The trace sampler is set to 1.0 (100% sampling) which may generate excessive telemetry data in production. Consider making the sampling rate configurable or using a lower default value for production environments.
use async_graphql::EmptySubscription;
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
} | ||
|
||
#[derive(InputObject)] | ||
#[derive(InputObject, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The Debug derive should be placed on its own line for consistency with other derive attributes in the codebase. Consider separating it: #[derive(InputObject)]
and #[derive(Debug)]
on separate lines.
#[derive(InputObject, Debug)] | |
#[derive(InputObject)] | |
#[derive(Debug)] |
Copilot uses AI. Check for mistakes.
.with_attributes(vec![ | ||
KeyValue::new(SERVICE_NAME, env!("CARGO_PKG_NAME")), | ||
KeyValue::new(SERVICE_VERSION, env!("CARGO_PKG_VERSION")), | ||
KeyValue::new(DEPLOYMENT_ENVIRONMENT_NAME, "develop"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deployment environment is hardcoded to 'develop'. This should be configurable through environment variables or the Config struct to support different deployment environments (development, staging, production).
Copilot uses AI. Check for mistakes.
.unwrap(); | ||
|
||
let reader = PeriodicReader::builder(exporter) | ||
.with_interval(std::time::Duration::from_secs(30)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The metrics export interval is hardcoded to 30 seconds. Consider making this configurable through environment variables or the Config struct to allow tuning for different environments.
Copilot uses AI. Check for mistakes.
closes #67