refactor logging logic by n0str · Pull Request #149 · codex-team/hawk.collector

n0str · 2026-02-11T15:27:01Z

No description provided.

Copilot

Pull request overview

This pull request refactors the logging infrastructure by migrating from the logrus library to Go's standard slog package with OpenTelemetry (OTLP) integration. The changes introduce a custom logger wrapper package and an otel package for structured logging and observability.

Changes:

Replaced sirupsen/logrus with a custom logger wrapper around slog with OpenTelemetry support
Added pkg/logger and pkg/otel packages with comprehensive test coverage
Updated all logging imports and calls across the entire codebase
Added environment configuration for OpenTelemetry logs endpoint and deployment environment
Updated Go dependencies and added OpenTelemetry SDK packages

Reviewed changes

Copilot reviewed 33 out of 35 changed files in this pull request and generated 19 comments.

Show a summary per file

File	Description
pkg/logger/logger.go	New logger wrapper providing compatibility layer between logrus-style APIs and slog
pkg/logger/logger_test.go	Unit tests for logger wrapper functionality
pkg/otel/otel.go	OpenTelemetry setup and slog handler implementation for OTLP export
pkg/otel/otel_test.go	Unit tests for OpenTelemetry handler and severity mapping
pkg/server/server.go	Updated logging imports and added project-scoped logging
pkg/server/server_test.go	New tests for HandleGenerateTestTimeSeries endpoint
pkg/server/releasehandler/*.go	Updated logging imports and patterns
pkg/server/releasehandler/multipart_test.go	New tests for getSingleFormValue function
pkg/server/errorshandler/*.go	Updated logging imports and added project-scoped logging
pkg/server/errorshandler/handler_test.go	New tests for determineQueue and getTimeSeriesKey functions
pkg/server/health.go	Updated logging import
pkg/redis/client.go	Updated logging import
pkg/periodic/periodic.go	Updated logging import
pkg/metrics/prometheus.go	Updated logging import
pkg/hawk/main.go	Updated logging import
pkg/broker/*.go	Updated logging imports
pkg/accounts/*.go	Updated logging imports and changed one error to debug level
cmd/collector/main.go	Refactored to initialize OTLP logging early and use structured logging
cmd/config.go	Added OpenTelemetry configuration fields
cmd/error.go	Updated error handling to use slog
go.mod	Updated Go version and added OpenTelemetry dependencies
go.sum	Updated dependency checksums
Dockerfile	Updated Go base image version
.env.sample, .env.docker	Added OTEL_LOGS_ENDPOINT and DEPLOYMENT_ENVIRONMENT variables
.gitignore	Added hawk.collector binary to ignore list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T17:07:48Z

pkg/server/server.go

 	// Generate test data
 	if err := s.ErrorsHandler.GenerateTestTimeSeriesData(reqBody.ProjectId); err != nil {
-		log.Errorf("Failed to generate test data: %v", err)
+		projectLogger.Error(fmt.Sprintf("Failed to generate test data: %v", err))


Similar issue: using fmt.Sprintf with structured logging is inefficient and loses the benefits of structured logging. Since projectId is already in the logger context, just pass the error as a structured field: projectLogger.Error("Failed to generate test data", "error", err) instead of formatting it into the message string.

Copilot · 2026-02-11T17:07:49Z

pkg/server/releasehandler/handler.go

 	}
-	log.Debugf("Found project with ID %s for integration token %s", projectId, token)
+	projectLogger := log.With("projectId", projectId)
+	projectLogger.Debug(fmt.Sprintf("Found project with ID %s for integration token %s", projectId, token))


This logging pattern defeats the purpose of structured logging. Instead of using fmt.Sprintf to embed values in the message string, pass them as structured fields. For example: projectLogger.Debug("Found project with ID for integration token", "integrationToken", integrationSecret). The projectId is already in the logger context.

Copilot · 2026-02-11T17:07:49Z

pkg/server/errorshandler/test_helpers.go

 	}

-	log.Infof("Test data generation completed for project %s", projectId)
+	projectLogger.Info(fmt.Sprintf("Test data generation completed for project %s", projectId))


Use structured logging: projectLogger.Info("Test data generation completed")

Suggested change

projectLogger.Info(fmt.Sprintf("Test data generation completed for project %s", projectId))

projectLogger.Info("Test data generation completed")

Copilot · 2026-02-11T17:07:50Z

cmd/config.go

+
+	// OpenTelemetry configuration
+	OTelLogsEndpoint string `env:"OTEL_LOGS_ENDPOINT"`
+	ServiceName      string `env:"SERVICE_NAME" defaultEnv:"collector"`


The env tag should use envDefault not defaultEnv for the caarlos0/env package. The correct tag is env:"SERVICE_NAME" envDefault:"collector". With the current incorrect tag, the default value will not be applied if the environment variable is not set.

Copilot · 2026-02-11T17:07:50Z

pkg/server/releasehandler/handler.go

 	projectLimits, ok := handler.AccountsMongoDBClient.GetProjectLimits(projectId)
 	if !ok {
-		log.Warnf("Project %s is not in the projects limits cache", projectId)
+		projectLogger.Warn(fmt.Sprintf("Project %s is not in the projects limits cache", projectId))


Use structured logging fields instead of fmt.Sprintf. Replace with: projectLogger.Warn("Project is not in the projects limits cache")

Suggested change

projectLogger.Warn(fmt.Sprintf("Project %s is not in the projects limits cache", projectId))

projectLogger.Warn("Project is not in the projects limits cache")

Copilot · 2026-02-11T17:07:53Z

pkg/server/errorshandler/test_helpers.go


 	// Hourly data: last 7 days (168 hours)
-	log.Infof("Generating hourly test data for project %s...", projectId)
+	projectLogger.Info(fmt.Sprintf("Generating hourly test data for project %s...", projectId))


Use structured logging: projectLogger.Info("Generating hourly test data")

Suggested change

projectLogger.Info(fmt.Sprintf("Generating hourly test data for project %s...", projectId))

projectLogger.Info("Generating hourly test data")

Copilot · 2026-02-11T17:07:54Z

pkg/server/errorshandler/handler.go

@@ -79,7 +80,7 @@ func (handler *Handler) process(body []byte) ResponseMessage {

 	rateWithinLimit, err := handler.RedisClient.UpdateRateLimit(projectId, projectLimits.EventsLimit, projectLimits.EventsPeriod)
 	if err != nil {
-		log.Errorf("Failed to update rate limit: %s", err)
+		projectLogger.Error(fmt.Sprintf("Failed to update rate limit: %s", err))
 		return ResponseMessage{402, true, "Failed to update rate limit"}
 	}
 	if !rateWithinLimit {
@@ -103,7 +104,7 @@ func (handler *Handler) process(body []byte) ResponseMessage {

 	// send serialized message to a broker
 	brokerMessage := broker.Message{Payload: rawMessage, Route: handler.determineQueue(message.CatcherType)}
-	log.Debugf("Send to queue: %s", brokerMessage)
+	projectLogger.Debug(fmt.Sprintf("Send to queue: %s", brokerMessage))
 	handler.Broker.Chan <- brokerMessage


Use structured logging throughout this function: projectLogger.Debug("Found project with ID for integration token", "integrationToken", integrationSecret), projectLogger.Warn("Project is not in the projects limits cache"), projectLogger.Debug("Project limits retrieved", "limits", projectLimits), projectLogger.Error("Failed to update rate limit", "error", err), projectLogger.Debug("Send to queue", "message", brokerMessage)

Copilot · 2026-02-11T17:07:54Z

pkg/accounts/cache.go

 			validTokensTmp[integrationSecret] = project.ProjectID.Hex()
 		} else {
-			log.Errorf("Integration token %s is invalid: %s", project.Token, err)
+			log.Debugf("Integration token %s is invalid: %s", project.Token, err)


The log level was changed from Error to Debug for invalid integration tokens. This may hide important errors about invalid tokens in production. If invalid tokens are expected (e.g., for disabled projects), this change is appropriate. However, if they indicate a data integrity issue, this should remain at Error or Warn level to ensure visibility.

Suggested change

log.Debugf("Integration token %s is invalid: %s", project.Token, err)

log.Warnf("Integration token %s is invalid: %s", project.Token, err)

Copilot · 2026-02-11T17:07:54Z

pkg/server/releasehandler/handler.go


 			// append file name and content to files array
-			log.Debugf("[release] Got filename: %s", header.Filename)
+			projectLogger.Debug(fmt.Sprintf("[release] Got filename: %s", header.Filename))


Use structured logging: projectLogger.Debug("Got filename", "filename", header.Filename)

Suggested change

projectLogger.Debug(fmt.Sprintf("[release] Got filename: %s", header.Filename))

projectLogger.Debug("[release] Got filename", "filename", header.Filename)

Copilot · 2026-02-11T17:07:55Z

pkg/server/errorshandler/handler_sentry.go

 	rateWithinLimit, err := handler.RedisClient.UpdateRateLimit(projectId, projectLimits.EventsLimit, projectLimits.EventsPeriod)
 	if err != nil {
-		log.Errorf("Failed to update rate limit: %s", err)
+		projectLogger.Error(fmt.Sprintf("Failed to update rate limit: %s", err))


Use structured logging: projectLogger.Error("Failed to update rate limit", "error", err)

refactor logging logic

27962db

n0str requested a review from khaydarov as a code owner February 11, 2026 15:27

n0str added 2 commits February 11, 2026 19:45

remove logs

056cebc

make cache invalid project token error debug

73c0144

n0str requested review from Copilot, e11sy and slaveeks February 11, 2026 17:01

Copilot started reviewing on behalf of n0str February 11, 2026 17:01 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor logging logic#149

refactor logging logic#149
n0str wants to merge 3 commits intomasterfrom
otel-logs

n0str commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	projectLogger.Info(fmt.Sprintf("Test data generation completed for project %s", projectId))
	projectLogger.Info("Test data generation completed")

	projectLogger.Warn(fmt.Sprintf("Project %s is not in the projects limits cache", projectId))
	projectLogger.Warn("Project is not in the projects limits cache")

	projectLogger.Info(fmt.Sprintf("Generating hourly test data for project %s...", projectId))
	projectLogger.Info("Generating hourly test data")

	log.Debugf("Integration token %s is invalid: %s", project.Token, err)
	log.Warnf("Integration token %s is invalid: %s", project.Token, err)

	projectLogger.Debug(fmt.Sprintf("[release] Got filename: %s", header.Filename))
	projectLogger.Debug("[release] Got filename", "filename", header.Filename)

Conversation

n0str commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants