feat: Add Logrus package challenge series #527
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAdds a new Logrus learning path with four challenges (basic levels, structured request logging, advanced hooks, production patterns), each including documentation, metadata, go modules, test scaffolds, run scripts, solution templates, and example student submissions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Middleware
participant Handler
participant Logger
Note over Client,Server: Challenge 2 — request-scoped logger via middleware
Client->>Server: HTTP GET /hello (User-Agent)
Server->>Middleware: incoming request
Middleware->>Logger: uuid.New(), logger.WithFields(request_id,http_method,uri,user_agent)
Middleware->>Server: r.WithContext(ctx) -> next
Server->>Handler: handler receives request with ctx
Handler->>Logger: getLogger(ctx).WithField(user_id).Info("Processing hello request")
Handler-->>Client: 200 "Hello, world!"
sequenceDiagram
participant App
participant Logger
participant ErrorHook
participant ConsoleOut
participant HookOut
Note over App,ErrorHook: Challenge 3 — ErrorHook routes Error/Fatal JSON to separate writer
App->>Logger: logger.Info / logger.Warn / logger.Error
Logger->>ConsoleOut: TextFormatter writes Info/Warn/Error
alt level == Error or Fatal
Logger->>ErrorHook: Fire(entry)
ErrorHook->>HookOut: Formatter.Format(entry) + newline (JSON)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (30)
packages/logrus/challenge-2-structured-logging-and-fields/submissions/LeeFred3042U/solution.go (2)
11-13: Consider using a more collision-resistant context key type.Using
intas the context key type is less collision-resistant than using a string or custom struct type. While this works, it increases the risk of key collisions if other code also uses integer context keys.Apply this diff for a more robust approach:
-type loggerKeyType int +type loggerKeyType string -const loggerKey loggerKeyType = 0 +const loggerKey loggerKeyType = "logger"
54-54: Check error return from w.Write.The
w.Writecall returns an error that is currently ignored. While this is common in simple examples, checking the error is a best practice.Apply this diff to check the error:
- w.Write([]byte("Hello, world!")) + if _, err := w.Write([]byte("Hello, world!")); err != nil { + logger.WithError(err).Error("Failed to write response") + }packages/logrus/challenge-2-structured-logging-and-fields/solution-template.go (1)
8-8: Remove unused import.The
osimport is not used in the template or required for the solution. Consider removing it to keep the template clean.Apply this diff:
"context" "fmt" "log" "net/http" - "os" "github.com/google/uuid"packages/logrus/challenge-2-structured-logging-and-fields/solution-template_test.go (1)
67-68: Consider the brittleness of strict log line count.The test expects exactly 2 log lines. While this validates the precise logging behavior (one from middleware, one from handler), it's fragile if the implementation adds any additional logging (e.g., debug statements).
For an educational challenge where you're testing specific behavior, this strictness is reasonable. However, if the implementation evolves, you might consider using
require.GreaterOrEqual(t, len(logLines), 2)and then validating the first two entries specifically.packages/logrus/challenge-3-advanced-configuration-and-hooks/run_test.sh (1)
34-34: Consider removing error suppression from cp command.The
2>/dev/nullredirection silences errors when copyinggo.modandgo.sum. If these files are missing (which would be a problem), the error would be hidden, and later commands might fail with less clear error messages.Consider removing the error suppression or checking if the files exist first:
-cp "$SUBMISSION_FILE" "solution-template_test.go" "go.mod" "go.sum" "$TEMP_DIR/" 2>/dev/null +if ! cp "$SUBMISSION_FILE" "solution-template_test.go" "go.mod" "go.sum" "$TEMP_DIR/"; then + echo "Error: Failed to copy necessary files to temporary directory." + rm -rf "$TEMP_DIR" + exit 1 +fipackages/logrus/challenge-3-advanced-configuration-and-hooks/solution-template.go (2)
18-23: Verify learners understand this placeholder won't execute.The
nilreturn will cause issues if the hook is added to a logger and triggered, as logrus expects a valid slice. While appropriate for a template, ensure documentation or tests make clear this method must be implemented before the code can run successfully.Consider returning an empty slice instead to prevent potential panics during development:
- return nil + return []logrus.Level{} // TODO: Return ErrorLevel and FatalLevel
25-35: Consider demonstrating error handling in the hints.The hints at lines 28 and 32 show capturing errors but don't demonstrate proper error handling. Since this is an educational template teaching production logging patterns, the hints should model checking and returning errors from
h.Formatter.Format(entry).Consider updating the hints to include error handling:
// TODO: Use the hook's formatter to format the entry into bytes -// Hint: line, err := h.Formatter.Format(entry) +// Hint: line, err := h.Formatter.Format(entry) +// if err != nil { return err } // TODO: Write the formatted line to the hook's output writer (h.Out) // Remember to add a newline character at the end to separate log entries -// Hint: _, err = h.Out.Write(append(line, '\n')) +// Hint: if _, err := h.Out.Write(append(line, '\n')); err != nil { return err } +// return nilpackages/logrus/challenge-1-basic-logging-and-levels/learning.md (1)
29-33: Add language specifier to fenced code block.The code block showing example output should specify a language for proper syntax highlighting and rendering.
Apply this diff:
-``` +```text 2025/10/02 18:00:00 Hello from standard log</blockquote></details> <details> <summary>packages/logrus/challenge-1-basic-logging-and-levels/run_test.sh (1)</summary><blockquote> `34-34`: **Avoid silencing copy errors.** Redirecting stderr to `/dev/null` suppresses potential errors when copying files. If `go.sum` is missing or there's a permission issue, the script will silently continue and may fail later with a less clear error message. Consider removing `2>/dev/null` or adding explicit validation: ```diff -cp "$SUBMISSION_FILE" "solution-template_test.go" "go.mod" "go.sum" "$TEMP_DIR/" 2>/dev/null +cp "$SUBMISSION_FILE" "solution-template_test.go" "go.mod" "go.sum" "$TEMP_DIR/" || { + echo "Error: Failed to copy required files to temporary directory." + rm -rf "$TEMP_DIR" + exit 1 +}packages/logrus/challenge-1-basic-logging-and-levels/solution-template_test.go (5)
65-76: Strengthen expectations: error logs should appear at debug and info levels.At Debug and Info levels, Error messages pass the filter and should be asserted to improve coverage.
@@ { name: "Debug Level", levelToSet: "debug", - expectedLogs: []string{"Checking system status", "Logbook application starting up", "Disk space is running low"}, + expectedLogs: []string{"Checking system status", "Logbook application starting up", "Disk space is running low", "Failed to connect to remote backup service"}, unexpectedLogs: []string{}, }, { name: "Info Level", levelToSet: "info", - expectedLogs: []string{"Logbook application starting up", "Disk space is running low"}, + expectedLogs: []string{"Logbook application starting up", "Disk space is running low", "Failed to connect to remote backup service"}, unexpectedLogs: []string{"Checking system status"}, },
102-104: Prefer io.Discard; captureOutput overrides Out anyway.No need to route through Stdout here; keeps the test quieter if anything leaks.
- setupLogger(os.Stdout, tc.levelToSet) + setupLogger(io.Discard, tc.levelToSet)
171-186: Remove redundant ExitFunc override in panic test.At
level="panic", Fatal logs are suppressed, so overridingExitFuncis unnecessary noise.func TestPanicLogsPanic(t *testing.T) { defer func() { r := recover() assert.NotNil(t, r, "Expected a panic to occur") }() setupLogger(io.Discard, "panic") - - // Mock exit function to prevent it from terminating before panic - originalExitFunc := logrus.StandardLogger().ExitFunc - logrus.StandardLogger().ExitFunc = func(int) { /* Do nothing */ } - - defer func() { logrus.StandardLogger().ExitFunc = originalExitFunc }() runLogbookOperations() }
52-56: Add a case‑insensitive level parsing check.Users commonly pass uppercase levels; asserting this prevents regressions in
setupLogger.t.Run("defaults to info for invalid level", func(t *testing.T) { setupLogger(io.Discard, "invalid-level") assert.Equal(t, logrus.InfoLevel, logrus.GetLevel(), "Log level should default to Info for invalid input") }) + + t.Run("parses level case-insensitively", func(t *testing.T) { + setupLogger(io.Discard, "INFO") + assert.Equal(t, logrus.InfoLevel, logrus.GetLevel(), "Uppercase level should be parsed correctly") + })
24-33: Snapshot/restore global logger state to avoid cross‑test leakage.Consider stashing and restoring the standard logger state with
t.Cleanupfor safety across future parallel subtests.Add helpers:
type loggerState struct { out io.Writer level logrus.Level formatter logrus.Formatter exitFunc func(int) } func stashLogger() loggerState { l := logrus.StandardLogger() return loggerState{out: l.Out, level: l.Level, formatter: l.Formatter, exitFunc: l.ExitFunc} } func restoreLogger(s loggerState) { l := logrus.StandardLogger() l.SetOutput(s.out) l.SetLevel(s.level) l.SetFormatter(s.formatter) l.ExitFunc = s.exitFunc }Usage at test starts:
s := stashLogger() t.Cleanup(func(){ restoreLogger(s) })packages/logrus/challenge-4-production-logging-patterns/README.md (2)
64-69: Per-destination formatting requires hooks or multiple loggers (MultiWriter alone won’t do).Using io.MultiWriter with a single logger.Formatter applies one formatter to all writers. To get Text on stdout and JSON in app.log, add a writer hook (or a second logger). Example hook:
+// writerHook writes entries with its own formatter to a specific writer. +type writerHook struct { + Writer io.Writer + Formatter logrus.Formatter + levels []logrus.Level +} +func (h *writerHook) Levels() []logrus.Level { return h.levels } +func (h *writerHook) Fire(e *logrus.Entry) error { + b, err := h.Formatter.Format(e); if err != nil { return err } + _, err = h.Writer.Write(b); return err +} + func setupLogger() *logrus.Logger { l := logrus.New() - // MultiWriter to stdout+file and set TextFormatter - // (this would format both outputs the same) + // Console: text + l.SetOutput(os.Stdout) + l.SetFormatter(&logrus.TextFormatter{FullTimestamp: true}) + // File: JSON via hook + f, _ := os.OpenFile("app.log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o666) + l.AddHook(&writerHook{ + Writer: f, + Formatter: &logrus.JSONFormatter{}, + levels: logrus.AllLevels, + }) // RetryHook to stderr remains as described return l }
42-56: Align Task struct docs with scaffold/tests.README shows Task including Duration time.Duration, but the scaffold defines only ID, Name, Retries and logs duration as a field. Clarify that duration is a logged field (not necessarily a Task member) or add Duration consistently across docs/scaffold.
packages/logrus/challenge-4-production-logging-patterns/learning.md (1)
49-55: Note the single-formatter constraint of MultiWriter.This example sets TextFormatter; both stdout and app.log will be text. Add a note or show a writer hook to emit JSON to file while keeping text on console (see README comment).
packages/logrus/challenge-4-production-logging-patterns/run_test.sh (1)
1-9: Harden the script and address ShellCheck SC2164.
- Add set -Eeuo pipefail and a cleanup trap.
- Use read -r.
- Guard pushd/popd.
- Consider go test -count=1 to avoid cache surprises.
-#!/bin/bash +#!/usr/bin/env bash +set -Eeuo pipefail + +# Ensure temp dir is removed on exit +cleanup() { + [[ -n "${TEMP_DIR:-}" && -d "${TEMP_DIR:-}" ]] && rm -rf "$TEMP_DIR" +} +trap cleanup EXIT -# Prompt for GitHub username -read -p "Enter your GitHub username: " USERNAME +# Prompt for GitHub username +read -r -p "Enter your GitHub username: " USERNAME -# Navigate to the temporary directory -pushd "$TEMP_DIR" > /dev/null +# Navigate to the temporary directory +pushd "$TEMP_DIR" > /dev/null || { echo "pushd failed"; exit 1; } go mod tidy || { echo "Failed to tidy dependencies." - popd > /dev/null - rm -rf "$TEMP_DIR" + popd > /dev/null || true exit 1 } -echo "Executing tests..." -go test -v -cover +echo "Executing tests..." +go test -v -cover -count=1 -# Return to the original directory -popd > /dev/null +# Return to the original directory +popd > /dev/null || { echo "popd failed"; exit 1; } -# Clean up the temporary directory -rm -rf "$TEMP_DIR" +# Cleanup happens via trapAlso applies to: 41-51, 55-71
packages/logrus/challenge-4-production-logging-patterns/go.mod (1)
1-1: Optional: set a fully qualified module path.If you want ‘go get’ to work outside this repo, consider using your canonical module path (e.g., github.com//go-interview-practice/packages/logrus/challenge-4-production-logging-patterns).
packages/logrus/challenge-4-production-logging-patterns/hints.md (3)
8-21: Replace hard tabs with spaces to satisfy markdownlint (MD010).Convert tabs to spaces in code fences for consistency with repo style. Example:
-import ( - "io" - "github.com/sirupsen/logrus" -) +import ( + "io" + "github.com/sirupsen/logrus" +)
29-37: Avoid adding an extra newline in Fire; JSONFormatter already includes it.Prevent blank lines between records.
-_, err = h.Out.Write(append(line, '\n')) +_, err = h.Out.Write(line)
47-54: Prefer failing clearly without panic in snippets.Swap panic for a clear fatal log (or propagate the error in real code):
-logFile, err := os.OpenFile("app.log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) -if err != nil { - panic("Failed to open log file") -} +logFile, err := os.OpenFile("app.log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o666) +if err != nil { + log.Fatalf("failed to open log file: %v", err) +}packages/logrus/challenge-4-production-logging-patterns/solution-template.go (3)
10-21: Export RetryHook fields expected by tests and implement Levels.Tests reflect on Out and Formatter and require Warn/Error levels. Keep behavior minimal but compile-friendly.
type RetryHook struct { - // TODO: define fields (e.g., Out, Formatter) + Out io.Writer + Formatter logrus.Formatter } // Levels tells Logrus which severities this hook should fire for // TODO: Restrict to Warn and Error func (h *RetryHook) Levels() []logrus.Level { - // TODO: return the correct levels - return nil + return []logrus.Level{logrus.WarnLevel, logrus.ErrorLevel} }
23-28: Implement Fire to format and write; keep it simple.func (h *RetryHook) Fire(entry *logrus.Entry) error { - // TODO: implement hook writing logic - return nil + if h == nil || h.Out == nil || h.Formatter == nil { + return nil + } + b, err := h.Formatter.Format(entry) + if err != nil { return err } + _, err = h.Out.Write(b) + return err }
3-8: Remove unused import to avoid compile warnings in the scaffold.-import ( - "os" - "time" - - "github.com/sirupsen/logrus" -) +import ( + "time" + "github.com/sirupsen/logrus" +)packages/logrus/challenge-4-production-logging-patterns/solution-template_test.go (2)
22-41: Mark helpers and raise scanner limit for large JSON lines.-func parseJSONLines(r io.Reader) ([]logLine, error) { +func parseJSONLines(r io.Reader) ([]logLine, error) { + testingT := testing.T{} // placeholder not used; t.Helper used in callers instead if desired var out []logLine - scanner := bufio.NewScanner(r) + scanner := bufio.NewScanner(r) + scanner.Buffer(make([]byte, 0, 1024), 1024*1024) // up to 1MB linesAlso consider adding t.Helper() to parseJSONLines and findRetryHook if you pass t in; otherwise, keep as-is.
43-57: Mark findRetryHook as a test helper (optional).If you thread through t, call t.Helper() to improve failure locations. Otherwise fine.
packages/logrus/challenge-4-production-logging-patterns/submissions/LeeFred3042U/solution.go (3)
93-101: File handling: avoid rename-on-start, handle errors, and prevent panics.Rename + append can make tests nondeterministic and silently drop rename errors; panic on open is harsh. Prefer truncating the file and degrade gracefully if open fails. Also, consider lifecycle management for the file handle.
Apply this diff:
- logFile := "app.log" - if _, err := os.Stat(logFile); err == nil { - os.Rename(logFile, fmt.Sprintf("%s.old", logFile)) - } - file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) - if err != nil { - panic(fmt.Sprintf("could not open log file: %v", err)) - } + logFile := "app.log" + file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + fmt.Fprintf(os.Stderr, "could not open log file %q: %v; continuing with stdout only\n", logFile, err) + file = nil + }Note: For long‑running processes, return a cleanup func to close the file, or close it in main when done.
45-76: Make log fields consistent; add event and numeric duration.Unify structured fields across all entries and emit duration as milliseconds for machine parsing.
Apply this diff:
func runWorker(logger *logrus.Logger, tasks []Task) { for _, task := range tasks { start := time.Now() - // Start log - logger.WithField("task_id", task.ID). - Infof("Starting task: %s", task.Name) + // Base entry for this task + entry := logger.WithFields(logrus.Fields{ + "task_id": task.ID, + "task_name": task.Name, + }) + // Start log + entry.WithField("event", "start"). + Infof("Starting task: %s", task.Name) // Retry log if task.Retries > 0 { - logger.WithFields(logrus.Fields{ - "task_id": task.ID, - "retries": task.Retries, - }).Warnf("Task %s retried %d time(s)", task.Name, task.Retries) + entry.WithFields(logrus.Fields{ + "event": "retry", + "retries": task.Retries, + }).Warnf("Task %s retried %d time(s)", task.Name, task.Retries) } // Failure simulation if task.Name == "FailMe" { - logger.WithFields(logrus.Fields{ - "task_id": task.ID, - "error": "simulated failure", - }).Error("Task failed due to simulated error") + entry.WithFields(logrus.Fields{ + "event": "failure", + "error": "simulated failure", + }).Error("Task failed due to simulated error") continue } // Success log - duration := time.Since(start) - logger.WithFields(logrus.Fields{ - "task_id": task.ID, - "duration": duration, - }).Infof("Task %s completed successfully", task.Name) + duration := time.Since(start) + entry.WithFields(logrus.Fields{ + "event": "success", + "duration_ms": duration.Milliseconds(), + }).Infof("Task %s completed successfully", task.Name) } }
86-91: Color output: gate colors by TTY (optional).To avoid escape codes in non‑TTY environments, set colors only when stdout is a terminal.
Example (requires github.com/mattn/go-isatty):
isTTY := isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd()) consoleFormatter := &logrus.TextFormatter{ DisableColors: !isTTY, FullTimestamp: true, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
packages/logrus/challenge-1-basic-logging-and-levels/go.sumis excluded by!**/*.sumpackages/logrus/challenge-2-structured-logging-and-fields/go.sumis excluded by!**/*.sumpackages/logrus/challenge-3-advanced-configuration-and-hooks/go.sumis excluded by!**/*.sumpackages/logrus/challenge-4-production-logging-patterns/app.logis excluded by!**/*.logpackages/logrus/challenge-4-production-logging-patterns/go.sumis excluded by!**/*.sum
📒 Files selected for processing (37)
packages/logrus/challenge-1-basic-logging-and-levels/README.md(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/go.mod(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/hints.md(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/learning.md(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/metadata.json(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/run_test.sh(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/solution-template.go(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/solution-template_test.go(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/submissions/LeeFred3042U/solution.go(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/README.md(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/go.mod(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/hints.md(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/learning.md(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/metadata.json(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/run_test.sh(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/solution-template.go(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/solution-template_test.go(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/submissions/LeeFred3042U/solution.go(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/README.md(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/go.mod(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/hints.md(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/learning.md(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/metadata.json(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/run_test.sh(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/solution-template.go(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/solution-template_test.go(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/submissions/LeeFred3042U/solution.go(1 hunks)packages/logrus/challenge-4-production-logging-patterns/README.md(1 hunks)packages/logrus/challenge-4-production-logging-patterns/go.mod(1 hunks)packages/logrus/challenge-4-production-logging-patterns/hints.md(1 hunks)packages/logrus/challenge-4-production-logging-patterns/learning.md(1 hunks)packages/logrus/challenge-4-production-logging-patterns/metadata.json(1 hunks)packages/logrus/challenge-4-production-logging-patterns/run_test.sh(1 hunks)packages/logrus/challenge-4-production-logging-patterns/solution-template.go(1 hunks)packages/logrus/challenge-4-production-logging-patterns/solution-template_test.go(1 hunks)packages/logrus/challenge-4-production-logging-patterns/submissions/LeeFred3042U/solution.go(1 hunks)packages/logrus/package.json(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/logrus/challenge-2-structured-logging-and-fields/learning.md
[style] ~38-~38: Consider a different adjective to strengthen your wording.
Context: ... | Stores deep context for debugging and replay; ideal...
(DEEP_PROFOUND)
[style] ~39-~39: Consider a different adjective to strengthen your wording.
Context: ... production; allows fast dashboards and deep investigations. ...
(DEEP_PROFOUND)
packages/logrus/challenge-2-structured-logging-and-fields/README.md
[style] ~76-~76: Consider a different adjective to strengthen your wording.
Context: ...red body: fields for indexing, body for deep context. | Fields: request_id=... Bod...
(DEEP_PROFOUND)
packages/logrus/challenge-3-advanced-configuration-and-hooks/learning.md
[style] ~14-~14: Consider using a different adverb to strengthen your wording.
Context: ... to another destination, potentially in a completely different format. The logrus.Hook in...
(COMPLETELY_ENTIRELY)
🪛 markdownlint-cli2 (0.18.1)
packages/logrus/challenge-3-advanced-configuration-and-hooks/README.md
190-190: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
packages/logrus/challenge-4-production-logging-patterns/learning.md
109-109: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/logrus/challenge-2-structured-logging-and-fields/learning.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
35-35: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
40-40: Spaces inside code span elements
(MD038, no-space-in-code)
packages/logrus/challenge-4-production-logging-patterns/hints.md
9-9: Hard tabs
Column: 1
(MD010, no-hard-tabs)
10-10: Hard tabs
Column: 1
(MD010, no-hard-tabs)
14-14: Hard tabs
Column: 1
(MD010, no-hard-tabs)
15-15: Hard tabs
Column: 1
(MD010, no-hard-tabs)
19-19: Hard tabs
Column: 1
(MD010, no-hard-tabs)
31-31: Hard tabs
Column: 1
(MD010, no-hard-tabs)
32-32: Hard tabs
Column: 1
(MD010, no-hard-tabs)
33-33: Hard tabs
Column: 1
(MD010, no-hard-tabs)
34-34: Hard tabs
Column: 1
(MD010, no-hard-tabs)
35-35: Hard tabs
Column: 1
(MD010, no-hard-tabs)
36-36: Hard tabs
Column: 1
(MD010, no-hard-tabs)
49-49: Hard tabs
Column: 1
(MD010, no-hard-tabs)
64-64: Hard tabs
Column: 1
(MD010, no-hard-tabs)
65-65: Hard tabs
Column: 1
(MD010, no-hard-tabs)
66-66: Hard tabs
Column: 1
(MD010, no-hard-tabs)
70-70: Hard tabs
Column: 1
(MD010, no-hard-tabs)
72-72: Hard tabs
Column: 1
(MD010, no-hard-tabs)
73-73: Hard tabs
Column: 1
(MD010, no-hard-tabs)
74-74: Hard tabs
Column: 1
(MD010, no-hard-tabs)
75-75: Hard tabs
Column: 1
(MD010, no-hard-tabs)
77-77: Hard tabs
Column: 1
(MD010, no-hard-tabs)
78-78: Hard tabs
Column: 1
(MD010, no-hard-tabs)
79-79: Hard tabs
Column: 1
(MD010, no-hard-tabs)
80-80: Hard tabs
Column: 1
(MD010, no-hard-tabs)
81-81: Hard tabs
Column: 1
(MD010, no-hard-tabs)
82-82: Hard tabs
Column: 1
(MD010, no-hard-tabs)
83-83: Hard tabs
Column: 1
(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1
(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1
(MD010, no-hard-tabs)
packages/logrus/challenge-3-advanced-configuration-and-hooks/hints.md
5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
packages/logrus/challenge-1-basic-logging-and-levels/learning.md
31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
packages/logrus/challenge-1-basic-logging-and-levels/run_test.sh
[warning] 43-43: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails.
(SC2164)
[warning] 49-49: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.
(SC2164)
[warning] 61-61: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.
(SC2164)
packages/logrus/challenge-3-advanced-configuration-and-hooks/run_test.sh
[warning] 42-42: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails.
(SC2164)
[warning] 48-48: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.
(SC2164)
[warning] 60-60: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.
(SC2164)
packages/logrus/challenge-2-structured-logging-and-fields/run_test.sh
[warning] 42-42: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails.
(SC2164)
[warning] 48-48: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.
(SC2164)
[warning] 60-60: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.
(SC2164)
packages/logrus/challenge-4-production-logging-patterns/run_test.sh
[warning] 42-42: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails.
(SC2164)
[warning] 48-48: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.
(SC2164)
[warning] 60-60: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.
(SC2164)
🔇 Additional comments (31)
packages/logrus/package.json (1)
5-5: I've confirmed the version v1.9.3 is current. Now let me verify the GitHub stars count mentioned in the review comment.Update GitHub stars count from 24000 to approximately 25.5k.
The current star count is 25.5k, while the review comment references 24000. The version v1.9.3 remains the latest release, so no version update is needed.
packages/logrus/challenge-2-structured-logging-and-fields/learning.md (1)
1-149: Excellent educational content!The learning material is comprehensive, well-structured, and covers the key concepts progressively. The inclusion of the structured logging comparison table (lines 35-42) and practical examples with middleware setup (lines 52-72) provides strong educational value for learners.
packages/logrus/challenge-2-structured-logging-and-fields/metadata.json (1)
1-47: Well-structured challenge metadata.The metadata comprehensively defines the challenge scope, learning objectives, and requirements. The progressive difficulty (Intermediate) and estimated time (30-45 min) align well with the complexity of implementing middleware-based structured logging.
packages/logrus/challenge-2-structured-logging-and-fields/README.md (1)
1-88: Clear and comprehensive challenge specification.The README provides excellent guidance with specific implementation requirements, expected outputs, and testing criteria. The inclusion of both JSON and Text formatter examples (lines 31-41) helps learners understand the dual-format support requirement.
packages/logrus/challenge-2-structured-logging-and-fields/hints.md (1)
1-107: Excellent progressive hints for learners.The hints provide a well-structured learning path, breaking down the implementation into manageable steps. The code examples are clear and demonstrate best practices like checking type assertions (lines 64-71).
packages/logrus/challenge-2-structured-logging-and-fields/solution-template.go (2)
22-48: Clear TODOs guide learners effectively.The TODO comments provide clear guidance for implementing the middleware, with helpful hints about which functions to use and the expected flow. The structure correctly demonstrates the middleware pattern.
5-8: Remove unused imports from the template.The template includes unused imports (
fmt,log,os) that may confuse learners. Since these aren't used in the template or required for the solution, they should be removed.Apply this diff to remove unused imports:
import ( "context" - "fmt" - "log" "net/http" - "os" "github.com/google/uuid" "github.com/sirupsen/logrus" )Note: If learners need
logfor line 88, they can add it back. The current code useslogrus.Infoandlog.Fatal, sologis actually used on line 88. Let me reconsider.Actually, looking at line 88:
log.Fatal(err)- sologIS used. And line 70 has a hint aboutfmt.Fprintln, sofmtwill be needed. Theosimport appears unused though. Let me revise.Likely an incorrect or invalid review comment.
packages/logrus/challenge-2-structured-logging-and-fields/go.mod (2)
14-14: golang.org/x/sys version is already patched for known vulnerabilities.The version in use (v0.0.0-20220715151400-c0bba94af5f8, July 2022) is newer than the patched version for the reported MODERATE severity vulnerability (v0.0.0-20220412211240-33da011f77ad, April 2022). While the dependency is old, the current version already includes the fix for the "Incorrect privilege reporting in syscall" vulnerability.
However, updating to a recent version of golang.org/x/sys remains a good practice for receiving ongoing security updates and bug fixes, even for indirect dependencies.
3-3: ****Go 1.25.1 is a valid, released version of Go. As of October 19, 2025, the latest stable version is Go 1.25.3, but declaring Go 1.25.1 in the go.mod file is appropriate. The original concern that this version "may not be released yet" is incorrect.
Likely an incorrect or invalid review comment.
packages/logrus/challenge-2-structured-logging-and-fields/solution-template_test.go (2)
3-15: LGTM!The imports are well-organized and appropriate for testing HTTP middleware with structured logging.
17-33: LGTM!The
logEntrystruct correctly models the expected JSON log structure, and theisValidUUIDhelper provides proper UUID validation using the google/uuid library.packages/logrus/challenge-3-advanced-configuration-and-hooks/learning.md (1)
1-132: Excellent educational content!The learning material is well-structured, technically accurate, and provides clear explanations of the
logrus.Hookinterface. The progression from concept to implementation is pedagogically sound, with concrete code examples that align with the challenge requirements. The email BCC analogy effectively clarifies the hook concept.packages/logrus/challenge-3-advanced-configuration-and-hooks/metadata.json (1)
1-41: Well-structured challenge metadata.The metadata is comprehensive and provides clear learning objectives, prerequisites, and requirements. The real-world connection effectively motivates the challenge, and the bonus points encourage learners to explore advanced patterns.
packages/logrus/challenge-3-advanced-configuration-and-hooks/solution-template_test.go (2)
21-79: Comprehensive integration test with good coverage.The test effectively validates the dual-logging setup by:
- Capturing both main logger and hook output separately
- Verifying message content and structured fields
- Validating JSON parsing and structure
- Ensuring the hook filters correctly (errors only, not info/warn)
The test design is clean and assertions are specific.
81-89: Good unit test for level filtering.This focused test validates the hook's
Levels()method returns exactly the expected set of log levels. The assertions cover both positive cases (Error, Fatal) and negative cases (Warn, Info), plus cardinality.packages/logrus/challenge-3-advanced-configuration-and-hooks/hints.md (1)
1-100: Well-crafted hints that guide without over-explaining.The hints provide just enough guidance to help learners implement the solution while encouraging them to think through the problem. The code snippets are correct and the emphasis on exact message formats is helpful for passing tests.
packages/logrus/challenge-3-advanced-configuration-and-hooks/README.md (1)
1-228: Comprehensive and well-documented challenge instructions.The README provides excellent guidance with:
- Clear requirements and sample outputs
- Complete, copy-pasteable code examples
- Explicit testing requirements with exact field names
- Helpful notes about testability and synchronous behavior
The level of detail is appropriate for an intermediate learning challenge.
packages/logrus/challenge-3-advanced-configuration-and-hooks/submissions/LeeFred3042U/solution.go (2)
1-82: Correct and complete reference implementation.This solution properly implements the challenge requirements:
ErrorHookcorrectly implementslogrus.HookinterfaceLevels()filters to Error and Fatal onlyFire()formats with trailing newline as expected by testsrunTaskScheduler()logs with exact message formats and field namesmain()wires up the dual-logging setup correctlyThe implementation serves as a good reference for learners.
63-63: Note: Hook output won't be visible when running main.The
hookWriteris a localbytes.Buffer, so error logs written to the hook won't be visible when running the program. This is intentional for testing (tests inject their own buffer), but in a production scenario, you'd use a file or other persistent writer.The commented-out code at lines 80-81 shows awareness of this limitation.
packages/logrus/challenge-3-advanced-configuration-and-hooks/go.mod (2)
3-3: The review comment is incorrect and should be ignored.Go 1.25 was released on August 12, 2025, and is the latest stable Go release as of October 2025. The code's declaration of
go 1.25.1uses a current Go version and is valid. Go patch releases (such as 1.25.1) are standard in Go's versioning scheme. The suggested downgrade togo 1.23would be regressive rather than corrective.Likely an incorrect or invalid review comment.
6-14: Dependencies are current—no changes required.The direct dependencies in go.mod match the latest released versions: logrus v1.9.3 (released 22 Jun 2025) and testify v1.11.1 (released 27 Aug 2025). The older
golang.org/x/systransitive dependency reflects the versions logrus v1.9.3 was built with and does not require action.packages/logrus/challenge-3-advanced-configuration-and-hooks/solution-template.go (2)
1-16: LGTM! Clean struct definition for the custom hook.The imports are appropriate and the
ErrorHookstruct is well-designed with the necessary fields to implement thelogrus.Hookinterface.
37-55: Function structure is appropriate for the template.The scaffolding with clear TODOs and hints provides good guidance for learners. However, note that line 39 will execute when called, which creates a dependency on a properly initialized logger (see critical issue in
main()).packages/logrus/challenge-1-basic-logging-and-levels/hints.md (1)
1-98: LGTM!The hints are well-structured and provide clear, progressive guidance for learners. The sequential approach effectively scaffolds the learning experience.
packages/logrus/challenge-1-basic-logging-and-levels/metadata.json (1)
1-42: LGTM!The metadata is comprehensive and well-structured, providing clear learning objectives and requirements for the challenge.
packages/logrus/challenge-1-basic-logging-and-levels/README.md (1)
1-75: LGTM!The README provides clear, comprehensive guidance for the challenge with well-defined requirements and helpful sample outputs.
packages/logrus/challenge-1-basic-logging-and-levels/go.mod (1)
3-3: Go version 1.25.1 is valid.Go 1.25.2 is the latest stable release as of October 19, 2025, which means Go 1.25.1 is a legitimate prior release in the same version series. Using a recent but not latest minor release is standard practice and will not cause issues for learners.
Likely an incorrect or invalid review comment.
packages/logrus/challenge-4-production-logging-patterns/submissions/LeeFred3042U/solution.go (4)
36-41: LGTM: Simple, clear Task model.Struct is minimal and adequate for the exercise.
21-23: No action needed — code conforms to specification.The submitted solution correctly returns only
WarnLevelandErrorLevel. The challenge specification (README.md line 71) explicitly requires this behavior: "Levels(): ReturnWarnLevelandErrorLevel". The test file (solution-template_test.go line 46) verifies hooks are registered at exactly these two levels. AddingFatalLevelandPanicLevelwould violate the specification and break tests.Likely an incorrect or invalid review comment.
121-131: No import conflict — submission and test are in separate package scopes.The submission file is in
submissions/LeeFred3042U/, which forms a separate package namespace from the test file at the parent level. The test importssetupLogger()andrunWorker()fromsolution-template.go(same directory as the test), not from the submission. The submission'spackage mainis appropriate for a standalone executable and does not conflict with the test.
102-109: Review comment is incorrect: it violates the challenge requirement to useio.MultiWriter.The challenge README (line 65) explicitly requires using
io.MultiWriterto send logs to both console and file. Your proposed diff removes this requirement entirely, replacing it with separate hooks. While you correctly identify that MultiWriter + TextFormatter produces text (not JSON) in the file—contradicting the README's JSON requirement—the solution is to add a second file hook for JSON, not to abandon MultiWriter.Since the tests do not validate
app.logfile format, the current implementation passes all tests despite the spec contradiction. The correct feedback should either:
- Acknowledge the spec is contradictory and suggest using a separate file hook in addition to MultiWriter (keeping the MultiWriter requirement), or
- Note that tests don't validate file format, so the implementation satisfies the tests even if it violates the README.
Removing MultiWriter violates the challenge's explicit requirements.
Likely an incorrect or invalid review comment.
packages/logrus/challenge-1-basic-logging-and-levels/run_test.sh
Outdated
Show resolved
Hide resolved
packages/logrus/challenge-1-basic-logging-and-levels/solution-template_test.go
Show resolved
Hide resolved
packages/logrus/challenge-1-basic-logging-and-levels/solution-template.go
Outdated
Show resolved
Hide resolved
packages/logrus/challenge-1-basic-logging-and-levels/submissions/LeeFred3042U/solution.go
Outdated
Show resolved
Hide resolved
packages/logrus/challenge-2-structured-logging-and-fields/run_test.sh
Outdated
Show resolved
Hide resolved
packages/logrus/challenge-4-production-logging-patterns/learning.md
Outdated
Show resolved
Hide resolved
packages/logrus/challenge-4-production-logging-patterns/metadata.json
Outdated
Show resolved
Hide resolved
packages/logrus/challenge-4-production-logging-patterns/submissions/LeeFred3042U/solution.go
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/logrus/challenge-3-advanced-configuration-and-hooks/run_test.sh (1)
31-31: Add validation thatmktempsucceeded.While rare,
mktempcan fail in resource-constrained or restricted environments. Verify success before proceeding.Apply this diff:
-TEMP_DIR=$(mktemp -d) +TEMP_DIR=$(mktemp -d) || { + echo "Error: Failed to create temporary directory." + exit 1 +}packages/logrus/challenge-1-basic-logging-and-levels/submissions/LeeFred3042U/solution.go (1)
23-31: Optional: clarify Fatal/Panic behavior for learners.Calling Fatal exits (unless ExitFunc is mocked) and Panic raises; consider a brief comment noting this, since running the binary will terminate before Panic. No code change required.
packages/logrus/challenge-1-basic-logging-and-levels/run_test.sh (1)
1-9: Harden the script: strict mode, guarded cp, and guaranteed cleanup.Improve resilience and diagnostics without changing behavior.
-#!/bin/bash +#!/bin/bash +set -Eeuo pipefail + +# Ensure cleanup on exit +cleanup() { + rm -rf "${TEMP_DIR:-}" +} +trap cleanup EXIT @@ -cp "$SUBMISSION_FILE" "solution-template_test.go" "go.mod" "go.sum" "$TEMP_DIR/" 2>/dev/null +cp "$SUBMISSION_FILE" "solution-template_test.go" "go.mod" "go.sum" "$TEMP_DIR/" || { + echo "Failed to copy required files into temp workspace." + exit 1 +} @@ -# Clean up the temporary directory -rm -rf "$TEMP_DIR" +# Cleanup handled by trapAlso applies to: 33-36, 67-69
packages/logrus/challenge-1-basic-logging-and-levels/solution-template_test.go (2)
66-90: Keep messages in one place to avoid drift with submissions.Define expected messages as consts (or share from template) to keep punctuation/wording consistent across updates and docs.
105-115: Nit: setup output to io.Discard before capture.Since captureOutput overrides logger output, passing io.Discard to setup reduces incidental writes and clarifies intent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/logrus/challenge-3-advanced-configuration-and-hooks/go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
packages/logrus/challenge-1-basic-logging-and-levels/run_test.sh(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/solution-template.go(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/solution-template_test.go(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/submissions/LeeFred3042U/solution.go(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/run_test.sh(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/solution-template_test.go(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/submissions/LeeFred3042U/solution.go(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/go.mod(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/run_test.sh(1 hunks)packages/logrus/challenge-3-advanced-configuration-and-hooks/solution-template.go(1 hunks)packages/logrus/challenge-4-production-logging-patterns/learning.md(1 hunks)packages/logrus/challenge-4-production-logging-patterns/metadata.json(1 hunks)packages/logrus/challenge-4-production-logging-patterns/run_test.sh(1 hunks)packages/logrus/challenge-4-production-logging-patterns/submissions/LeeFred3042U/solution.go(1 hunks)packages/logrus/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/logrus/challenge-4-production-logging-patterns/metadata.json
- packages/logrus/challenge-3-advanced-configuration-and-hooks/solution-template.go
- packages/logrus/challenge-4-production-logging-patterns/submissions/LeeFred3042U/solution.go
- packages/logrus/challenge-4-production-logging-patterns/run_test.sh
- packages/logrus/challenge-1-basic-logging-and-levels/solution-template.go
- packages/logrus/challenge-2-structured-logging-and-fields/submissions/LeeFred3042U/solution.go
🔇 Additional comments (22)
packages/logrus/challenge-3-advanced-configuration-and-hooks/go.mod (3)
3-3: Verify Go version target.Go 1.24.0 appears to be a future or unreleased version. As of the knowledge cutoff (March 2025), Go 1.23.x is the latest stable release. Ensure this version specification is intentional and compatible with your CI/build environment, or update to a stable version like
1.23.
5-8: Dependency selection looks good.The direct dependencies (logrus v1.9.3 and testify v1.11.1) are reasonable choices for a structured logging challenge with testing support. Versions appear stable and well-maintained.
10-15: Indirect dependencies are properly declared.Transitive dependencies are correctly marked as indirect. The set of indirect dependencies (go-spew, go-difflib, sys, yaml.v3) is typical for logrus + testify stacks.
packages/logrus/challenge-4-production-logging-patterns/learning.md (1)
1-167: ✅ Markdown formatting issue resolved; content is accurate and well-structured.The previous MD040 linting issue (missing language tag on the console output block) has been fixed—line 109 now correctly shows
```text. All code fences now have appropriate language identifiers.The learning material is technically sound:
io.MultiWriterusage (lines 38–55) correctly demonstrates multi-destination logging.- Custom
RetryHookimplementation (lines 66–103) follows logrus patterns and shows proper filtering of log levels.- ELK stack explanation aligns with production logging best practices.
- The five best practices (lines 141–147) are practical and current for microservice logging.
The content progressively builds from concepts to concrete examples to real-world deployment patterns, making it suitable for the "Advanced" Challenge 4 difficulty level.
packages/logrus/package.json (3)
18-27: Tags and real-world usage coverage looks solid.The tags and real-world usage sections comprehensively cover observability and production scenarios, aligning well with the challenge progression from basic to advanced patterns.
12-17: <!-- [scratchpad_update]
[task review_file_1/1]
Verification complete - all checks passed[observations]
- Script output confirmed:
- All 4 directories exist: challenge-1-basic-logging-and-levels, challenge-2-structured-logging-and-fields, challenge-3-advanced-configuration-and-hooks, challenge-4-production-logging-patterns
- Identifiers in package.json match directory names exactly
- No mismatches found
- Logrus version v1.9.3 confirmed as valid version (stable, still maintained for security/bug fixes)
- File is new metadata file with appropriate structure
[analysis]
- Past critical issue about identifier/directory mismatch has been FIXED
- Current learning_path identifiers in package.json are correct
- All metadata appears valid and consistent
- No other issues detected
[done]
Verification complete: All checks passed. Original review comment flag about directory/identifier mismatch is resolved.
[scratchpad_end] -->No issues found—all learning_path identifiers match actual directories.
The verification confirms that the identifiers in
learning_path(lines 13-16) correspond exactly to the actual challenge directories inpackages/logrus/. The past critical issue regarding identifier mismatches has been resolved. All metadata is correct and consistent.
1-11: Update the Logrus star count from 24,000 to approximately 25,434.As of October 20, 2025, Logrus has approximately 25,434 GitHub stars, making the current value of
24000outdated. The versionv1.9.3is confirmed as current, and the GitHub URL appears correct.Note: The documentation URL and prerequisites cannot be fully verified through web search; these should be manually confirmed to align with the challenge content.
packages/logrus/challenge-2-structured-logging-and-fields/solution-template_test.go (7)
1-15: LGTM!The package declaration and imports are appropriate for the test requirements. The external dependencies (uuid, logrus, testify) align with the challenge objectives.
17-27: LGTM!The
logEntrystruct correctly models the expected JSON log fields for both middleware and handler log entries. Using string types for all fields is appropriate for JSON unmarshaling and simplifies assertions.
29-33: LGTM!The UUID validation helper is correctly implemented using the google/uuid library. This provides reliable validation for request ID correlation checks.
40-46: Excellent - past issue resolved!The cleanup logic properly saves and restores the global logrus configuration. This addresses the concern from the previous review about global state mutation.
48-108: LGTM!The JSON formatter test is well-structured and comprehensive. It properly validates:
- HTTP response correctness
- JSON log structure and field values
- UUID format for request IDs
- Request correlation across middleware and handler logs
- Field propagation from middleware to handler context
The test coverage aligns well with the Challenge 2 learning objectives for structured logging and context propagation.
112-118: Excellent - past issue resolved!The cleanup logic properly saves and restores the global logrus configuration, matching the pattern in the JSON formatter test. This ensures test isolation.
120-160: LGTM!The text formatter test provides good coverage using substring matching, which is a pragmatic approach for validating unstructured text logs. The test confirms all key fields are present in the output without being overly brittle about exact formatting.
packages/logrus/challenge-2-structured-logging-and-fields/run_test.sh (6)
1-15: LGTM!The script initialization is solid with proper shebang and directory validation to ensure it runs in the correct context.
17-28: LGTM!The username prompt and submission validation provide clear error messages and prevent the script from proceeding with invalid input.
30-37: LGTM!The temporary directory setup isolates test execution and the file copying/renaming ensures the test harness can find the participant's solution.
42-46: Excellent - past issue resolved!The error handling for
pushdnow properly checks for failures and cleans up the temporary directory before exiting. This addresses the concern from the previous review.
48-55: LGTM!The dependency tidying includes proper error handling with cleanup. Using
popd > /dev/null || trueensures the script continues cleanup even if popd fails.
57-75: LGTM!The test execution and cleanup sequence is well-structured:
- Captures the test exit code before cleanup
- Returns to the original directory (with error suppression for robustness)
- Removes temporary files
- Provides clear success/failure feedback
- Exits with the test exit code for CI/automation compatibility
packages/logrus/challenge-1-basic-logging-and-levels/solution-template_test.go (1)
92-125: LGTM: time‑bounded wait prevents hangs on missing Fatal.The done+timeout approach is correct and avoids indefinite blocking.
packages/logrus/challenge-1-basic-logging-and-levels/submissions/LeeFred3042U/solution.go (1)
26-30: Fix log message text to match test expectations (remove trailing periods).Tests assert exact message membership; periods cause failures. Align with solution-template_test.go expected strings.
- logrus.Info("Logbook application starting up.") - logrus.Warn("Disk space is running low.") - logrus.Error("Failed to connect to remote backup service.") - logrus.Fatal("Critical configuration file 'config.yml' not found.") - logrus.Panic("Unhandled database connection issue.") + logrus.Info("Logbook application starting up") + logrus.Warn("Disk space is running low") + logrus.Error("Failed to connect to remote backup service") + logrus.Fatal("Critical configuration file 'config.yml' not found") + logrus.Panic("Unhandled database connection issue")Likely an incorrect or invalid review comment.
packages/logrus/challenge-3-advanced-configuration-and-hooks/run_test.sh
Outdated
Show resolved
Hide resolved
packages/logrus/challenge-3-advanced-configuration-and-hooks/run_test.sh
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CONTRIBUTING.md (1)
29-35: Fix markdown list indentation in Go Version Policy section.The Go Version Policy section has several formatting issues:
- List indentation: Lines 31–35 are over-indented. Unordered list items should start at column 0, and nested items should use 2-space indentation.
- Section hierarchy: The "## Go Version Policy" header is at the same level as "## How to Contribute" but should be a subsection ("###") since it appears within that section.
- Table of Contents: The new subsection is not referenced in the TOC (lines 5–17).
Apply this diff to fix the formatting:
-## Go Version Policy +### Go Version Policy - - The repository supports Go 1.19+ (README). - - CI workflows use **Go 1.25** (`.github/workflows/pr-tests.yml` sets `go-version: 1.25.0`). - - Some modules declare higher Go versions due to dependencies: - - Go 1.24 (`golang.org/x/sys v0.37.0` requires 1.24) - - Contributors using Go versions ≥1.19 should be aware that CI may run a newer Go version, which could produce warnings if older syntax or deprecated features are used. +- The repository supports Go 1.19+ (README). +- CI workflows use **Go 1.25** (`.github/workflows/pr-tests.yml` sets `go-version: 1.25.0`). +- Some modules declare higher Go versions due to dependencies: + - Go 1.24 (`golang.org/x/sys v0.37.0` requires 1.24) +- Contributors using Go versions ≥1.19 should be aware that CI may run a newer Go version, which could produce warnings if older syntax or deprecated features are used.Also update the Table of Contents to include the new subsection:
- [How to Contribute](#how-to-contribute) - [Submitting a Solution](#submitting-a-solution) + - [Go Version Policy](#go-version-policy) - [Adding a New Challenge](#adding-a-new-challenge)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/logrus/challenge-3-advanced-configuration-and-hooks/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
CONTRIBUTING.md(1 hunks)packages/logrus/challenge-1-basic-logging-and-levels/go.mod(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/go.mod(1 hunks)packages/logrus/challenge-2-structured-logging-and-fields/run_test.sh(1 hunks)packages/logrus/challenge-4-production-logging-patterns/go.mod(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/logrus/challenge-2-structured-logging-and-fields/run_test.sh
- packages/logrus/challenge-2-structured-logging-and-fields/go.mod
- packages/logrus/challenge-4-production-logging-patterns/go.mod
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~32-~32: The official name of this software platform is spelled with a capital “H”.
Context: ...ME). - CI workflows use Go 1.25 (.github/workflows/pr-tests.yml sets `go-versio...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
CONTRIBUTING.md
31-31: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
34-34: Unordered list indentation
Expected: 2; Actual: 5
(MD007, ul-indent)
35-35: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
🔇 Additional comments (2)
packages/logrus/challenge-1-basic-logging-and-levels/go.mod (2)
1-1: Module naming concern is incorrect; no namespace collisions exist.The original review claims "other challenges will have same module name, causing conflicts," but this is factually inaccurate. The logrus challenges have distinct module names:
- challenge-1:
logrus- challenge-2:
challenge-2-structured-logging-and-fields- challenge-3:
challenge-3-advanced-configuration-and-hooks- challenge-4:
challenge-4-production-logging-patternsA bare module name like
logrusis valid and does not conflict with the externalgithub.com/sirupsen/logruspackage—Go resolves modules by full import path. While using a prefixed name (e.g.,logrus-challenge-1) would align with the repo's naming pattern, this is a consistency preference rather than a functional issue.The golang.org/x/sys pseudo-version is an indirect dependency managed by logrus/testify; addressing it is not the challenge author's primary responsibility.
Likely an incorrect or invalid review comment.
13-13: Remove the claim about Go 1.21 incompatibility; it is factually incorrect.The pseudo-version
v0.0.0-20220715151400is compatible with Go 1.21. Go 1.21 allows building with older dependencies if they do not declare a newer go version requirement.Modern semantic versions (v0.37.0 and earlier) are available. Consider updating for code maintainability and alignment with current Go practices, but not due to incompatibility concerns.
Likely an incorrect or invalid review comment.
2 participants
Description
This PR introduces a comprehensive, four-part package challenge series for the
logrusstructured logging library. It fulfills all the requirements outlined in issue #474, providing a complete learning path from beginner concepts to advanced, production-ready logging patternsSummary of Changes
packages/logrus/README.md,learning.md,hints.md,solution-template.go,solution-template_test.go,metadata.json,go.mod/go.sum, andrun_tests.shpackage.jsonmetadata file to register thelogruschallenge series with the learning platformKey Highlights
learning.mdfile and ahints.mdfile to provide a rich, supportive learning experienceSelf-Review Checklist
package.jsonmetadata file is included and complete