Add Graylog logging configuration #252 by Alexj9837 · Pull Request #264 · DiamondLightSource/numtracker

Alexj9837 · 2026-03-11T13:12:53Z

#252

Summary

Add tracing-gelf layer to ship structured logs to a Graylog instance via GELF TCP
Expose --graylog <URL> and --logging-level <LEVEL> CLI flags (also via NUMTRACKER_GRAYLOG / NUMTRACKER_GRAYLOG_LOG_LEVEL env vars)
Add Graylog env var injection to Helm chart with numtracker.graylog.enabled/host/level values

src/cli/mod.rs

.vscode/launch.json

ZohebShaikh

Looks good only few minor things

src/logging.rs

ZohebShaikh · 2026-03-17T13:21:28Z

src/logging.rs

+                if errors.0.is_empty() {
+                    error!("Failed to connect to graylog - address lookup failed");
+                } else {
+                    warn!(
+                        "Connection to graylog {:?} closed: {:?}",
+                        handle.address(),
+                        errors
+                    );


errors.0 give Vec<SocketAddr,Error> docs

If there are errors you get a warning which I find difficult to understand
This is just a question to understand the code.

I don't know will it be very difficult to write unit test to check all the error cases

The gelf library is odd. There is a bit more explanation in the amygdala version here.

I'm not sure if the warn should also be an error but the no errors == error case caught me out as well so it might be worth including a similar comment here.

Adding a similar error message to errors.0 , to provide context.

- Extract GraylogOptions into its own struct, independent of TracingOptions - Remove .vscode/launch.json (to be added in a separate PR) - Remove version/build additional_field calls from Graylog logger - Switch let-else to if-let in init_graylog - Warn when Graylog URL has no port rather than silently defaulting - Add reconnect loop so dropped connections recover automatically - Improve connection error messages to distinguish DNS failure from TCP errors

codecov · 2026-03-30T13:59:52Z

Codecov Report

❌ Patch coverage is 78.57143% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.53%. Comparing base (4d5553c) to head (9092648).

Files with missing lines	Patch %	Lines
src/logging.rs	68.25%	20 Missing ⚠️
src/main.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #264      +/-   ##
==========================================
+ Coverage   76.35%   76.53%   +0.18%     
==========================================
  Files          13       13              
  Lines        1937     2033      +96     
==========================================
+ Hits         1479     1556      +77     
- Misses        458      477      +19

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tpoliaw

Looks pretty good, thanks for looking at it. A couple of changes could make the error handling a bit cleaner and keep it more consistent with what is already there though.

src/cli/mod.rs

tpoliaw · 2026-03-30T16:24:13Z

src/main.rs

 async fn main() -> Result<(), Box<dyn Error>> {
    let args = Cli::init();
-    let _ = logging::init(args.log_level(), args.tracing());
+    let _ = logging::init(args.log_level(), args.tracing(), &args.graylog);


We should probably handle errors here. Currently if any log setup fails you get no output and debugging why is a pain.

tpoliaw · 2026-03-30T16:25:07Z

Cargo.toml

 openidconnect = { version = "4.0.0", optional = true }
 toml = { version = "1.0.0", optional = true }
+tracing-gelf = "0.9.0"
+thiserror = "2.0.18"


Bringing in thiserror for a single error type might be an overkill. Implementing the error manually is not too much boilerplate.

removing thiserror.

tpoliaw · 2026-03-30T16:28:47Z

.gitignore

 trackers/
+
+# Local IDE settings
+.vscode/settings.json


Shouldn't be part of this PR - it's a good change but can go in at the same time as the launch.json, maybe as .vscode/ and then manually add the specific files.

removing it.

tpoliaw · 2026-03-30T16:45:42Z

src/logging.rs

+                if errors.0.is_empty() {
+                    error!("Failed to connect to graylog - address lookup failed");
+                } else {
+                    warn!(
+                        "Connection to graylog {:?} closed: {:?}",
+                        handle.address(),
+                        errors
+                    );


The gelf library is odd. There is a bit more explanation in the amygdala version here.

I'm not sure if the warn should also be an error but the no errors == error case caught me out as well so it might be worth including a similar comment here.

src/logging.rs

Alexj9837 marked this pull request as ready for review March 16, 2026 15:59

ZohebShaikh reviewed Mar 17, 2026

View reviewed changes

src/cli/mod.rs Outdated Show resolved Hide resolved

ZohebShaikh reviewed Mar 17, 2026

View reviewed changes

.vscode/launch.json Outdated Show resolved Hide resolved

ZohebShaikh reviewed Mar 17, 2026

View reviewed changes

src/logging.rs Outdated Show resolved Hide resolved

src/logging.rs Outdated Show resolved Hide resolved

src/logging.rs Outdated Show resolved Hide resolved

ZohebShaikh reviewed Mar 17, 2026

View reviewed changes

Alexj9837 force-pushed the numtracker_graylog branch from bda4336 to 831a990 Compare March 18, 2026 15:44

Alexj9837 added 8 commits March 30, 2026 14:47

Add Graylog GELF logging layer

d4cd088

Add Graylog configuration to Helm chart

3a5641f

Document Graylog logging configuration

f0bb43a

Add VSCode debug launch config

ff2a5fe

changed to align with tracing

700688b

removing launch.json

27aa07c

Regenerate Cargo.lock after rebase onto main

0a86a82

Alexj9837 force-pushed the numtracker_graylog branch from 831a990 to 0a86a82 Compare March 30, 2026 13:56

Alexj9837 added 2 commits March 30, 2026 15:02

Fix clippy: use is_err() instead of matches!(result, Err(_))

9c231c4

Fix nightly rustfmt formatting

9092648

tpoliaw requested changes Mar 30, 2026

View reviewed changes

Conversation

Alexj9837 commented Mar 11, 2026

Summary

Uh oh!

Uh oh!

Uh oh!

ZohebShaikh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZohebShaikh Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tpoliaw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ZohebShaikh Mar 17, 2026 •

edited

Loading

codecov bot commented Mar 30, 2026 •

edited

Loading