rmcguire/go-server-with-otel
Template
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

development #3

Merged
rmcguire merged 7 commits from development into main 2025-08-14 20:00:14 +00:00
Conversation 3 Commits 7 Files Changed 9 +99 -260
rmcguire commented 2025-08-14 15:50:04 +00:00
Owner
Copy Link
No description provided.
rmcguire added 5 commits 2025-08-14 15:50:04 +00:00
adds OpenTelemetry startup tracing, updates Go dependencies, and refines build configurations de7a11684b
fix Makefile rename 4933e04e6a
add changelog 762bdff465
bump chart bbc6190104
upgrade go-app, adjust gitignore 497d85cf1c
rmcguire commented 2025-08-14 15:50:22 +00:00
Author
Owner
Copy Link

  1. High-Level Summary:
    This pull request significantly enhances the application's observability by integrating OpenTelemetry tracing into the startup process, allowing for better insights into initialization phases and potential errors. It also includes comprehensive updates to Go module dependencies, ensuring the project leverages the latest library versions for improved stability and features. Furthermore, the changes refine build and development configurations, such as excluding generated protocol buffer files from live-reloading and automating application name updates in the source code. These updates collectively contribute to a more robust, maintainable, and observable application.

  2. Detailed Description:

    • .air.toml: The configuration for air live-reloading is updated to exclude the proto directory, preventing unnecessary rebuilds during development.
    • .gitignore: The entry for Go workspace files is generalized from go.work to go.work* to better handle various workspace file names.
    • CHANGELOG.md: A new changelog file is introduced, documenting the changes for version v0.5.0, including OpenTelemetry integration and dependency updates.
    • Makefile: A new sed command is added to the rename target, ensuring the appName constant in main.go is automatically updated when the application name changes.
    • go.mod: Go module dependencies are updated to newer versions, including protovalidate, go-app, grpc, and various OpenTelemetry-related packages, along with a replace directive for prometheus/otlptranslator.
    • go.sum: Checksums for the updated Go module dependencies are refreshed to reflect the changes in go.mod.
    • go.work.sum: This file, likely related to Go workspaces, has been removed.
    • helm/Chart.yaml: The Helm chart version is incremented to 0.1.1, and the application version is updated to v0.5.0 to align with the new features.
    • main.go: The application's main entry point is refactored to incorporate OpenTelemetry tracing for the startup sequence, capturing initialization events and errors within a dedicated span.
1. **High-Level Summary:** This pull request significantly enhances the application's observability by integrating OpenTelemetry tracing into the startup process, allowing for better insights into initialization phases and potential errors. It also includes comprehensive updates to Go module dependencies, ensuring the project leverages the latest library versions for improved stability and features. Furthermore, the changes refine build and development configurations, such as excluding generated protocol buffer files from live-reloading and automating application name updates in the source code. These updates collectively contribute to a more robust, maintainable, and observable application. 2. **Detailed Description:** * `.air.toml`: The configuration for `air` live-reloading is updated to exclude the `proto` directory, preventing unnecessary rebuilds during development. * `.gitignore`: The entry for Go workspace files is generalized from `go.work` to `go.work*` to better handle various workspace file names. * `CHANGELOG.md`: A new changelog file is introduced, documenting the changes for version `v0.5.0`, including OpenTelemetry integration and dependency updates. * `Makefile`: A new `sed` command is added to the `rename` target, ensuring the `appName` constant in `main.go` is automatically updated when the application name changes. * `go.mod`: Go module dependencies are updated to newer versions, including `protovalidate`, `go-app`, `grpc`, and various OpenTelemetry-related packages, along with a `replace` directive for `prometheus/otlptranslator`. * `go.sum`: Checksums for the updated Go module dependencies are refreshed to reflect the changes in `go.mod`. * `go.work.sum`: This file, likely related to Go workspaces, has been removed. * `helm/Chart.yaml`: The Helm chart version is incremented to `0.1.1`, and the application version is updated to `v0.5.0` to align with the new features. * `main.go`: The application's main entry point is refactored to incorporate OpenTelemetry tracing for the startup sequence, capturing initialization events and errors within a dedicated span.
rmcguire commented 2025-08-14 17:32:33 +00:00
Author
Owner
Copy Link

/issues

/issues
rmcguire commented 2025-08-14 17:33:02 +00:00
Author
Owner
Copy Link

High-Level Summary:
The changes introduce OpenTelemetry tracing for application startup, update Go module dependencies, and refine build configurations. The overall direction is positive for observability and project maintenance, though a few areas could be improved for robustness and clarity.

Code Issues:

  • File: go.mod

  • Line: 7

  • Issue: The replace directive for github.com/prometheus/otlptranslator points to a specific, unversioned commit hash (v0.0.0-20250717125610-8549f4ab4f8f). Relying on unversioned commits can lead to build reproducibility issues if the commit is rebased or deleted, and it tightly couples the project to a potentially unstable or temporary state of the dependency.

  • Suggestion: Clarify the reason for this replace directive. If it's a temporary workaround for an upstream bug, consider adding a comment explaining the rationale and a plan for its removal once a stable, versioned release is available. If it's a permanent fork, ensure it's properly documented.

  • File: main.go

  • Line: 62

  • Issue: The log variable is initialized from zerolog.Ctx(ctx) before the OpenTelemetry startup span is created and the context is updated. Consequently, the log message "App configuration prepared" (and any other logs before the log re-assignment on line 65) will not be associated with the new appName+".startup" span, potentially hindering full observability of the startup phase.

  • Suggestion: Reorder the initialization so that the log variable is re-assigned from the span-enriched context before any logging statements that should be part of the startup span.

    // ...
app.InitOTEL()
tracer := otel.GetTracer(ctx, appName)
ctx, span := tracer.Start(ctx, appName+".startup", trace.WithAttributes(
    attribute.Int("appServices", len(appServices)),
))

log := zerolog.Ctx(ctx) // Re-initialize log with the span-enriched context
log.Debug().Any("mergedConfig", serviceConfig).Msg("App configuration prepared")
// ...

  • File: main.go

  • Line: 70

  • Issue: The appName+".startup" span is explicitly ended at the very end of the main function. If an unhandled panic or a log.Fatal() call occurs after the span is started but before the explicit span.End() call (e.g., during service.MergeServices or app.MustRun()), the span might not be properly closed, leading to incomplete traces in some failure scenarios.

  • Suggestion: Use defer span.End() immediately after starting the span to ensure it's always closed, regardless of how the main function exits. This is a more robust pattern for resource management.

    tracer := otel.GetTracer(ctx, appName)
ctx, span := tracer.Start(ctx, appName+".startup", trace.WithAttributes(
    attribute.Int("appServices", len(appServices)),
))
defer span.End() // Ensure the span is always ended

  • File: main.go

  • Line: 22

  • Issue: The appName constant is hardcoded in main.go and then updated via a sed command in the Makefile's rename target. This creates a brittle coupling between the Go source code and the build script. If the sed pattern changes or the constant name changes, the Makefile might fail to update it, leading to inconsistencies.

  • Suggestion: For values that need to be configurable at build time (like an application name), consider using Go's ldflags to inject the value during compilation. This decouples the source code from the build script's sed operations and makes the build more robust.

    • Example:
      In main.go: var appName = "demo-app" (or var appName string)
      In Makefile: go build -ldflags "-X main.appName=$(APP)"
**High-Level Summary:** The changes introduce OpenTelemetry tracing for application startup, update Go module dependencies, and refine build configurations. The overall direction is positive for observability and project maintenance, though a few areas could be improved for robustness and clarity. **Code Issues:** * **File:** go.mod * **Line:** 7 * **Issue:** The `replace` directive for `github.com/prometheus/otlptranslator` points to a specific, unversioned commit hash (`v0.0.0-20250717125610-8549f4ab4f8f`). Relying on unversioned commits can lead to build reproducibility issues if the commit is rebased or deleted, and it tightly couples the project to a potentially unstable or temporary state of the dependency. * **Suggestion:** Clarify the reason for this `replace` directive. If it's a temporary workaround for an upstream bug, consider adding a comment explaining the rationale and a plan for its removal once a stable, versioned release is available. If it's a permanent fork, ensure it's properly documented. * **File:** main.go * **Line:** 62 * **Issue:** The `log` variable is initialized from `zerolog.Ctx(ctx)` before the OpenTelemetry startup span is created and the context is updated. Consequently, the log message "App configuration prepared" (and any other logs before the `log` re-assignment on line 65) will not be associated with the new `appName+".startup"` span, potentially hindering full observability of the startup phase. * **Suggestion:** Reorder the initialization so that the `log` variable is re-assigned from the span-enriched context *before* any logging statements that should be part of the startup span. ```go // ... app.InitOTEL() tracer := otel.GetTracer(ctx, appName) ctx, span := tracer.Start(ctx, appName+".startup", trace.WithAttributes( attribute.Int("appServices", len(appServices)), )) log := zerolog.Ctx(ctx) // Re-initialize log with the span-enriched context log.Debug().Any("mergedConfig", serviceConfig).Msg("App configuration prepared") // ... ``` * **File:** main.go * **Line:** 70 * **Issue:** The `appName+".startup"` span is explicitly ended at the very end of the `main` function. If an unhandled panic or a `log.Fatal()` call occurs *after* the span is started but *before* the explicit `span.End()` call (e.g., during `service.MergeServices` or `app.MustRun()`), the span might not be properly closed, leading to incomplete traces in some failure scenarios. * **Suggestion:** Use `defer span.End()` immediately after starting the span to ensure it's always closed, regardless of how the `main` function exits. This is a more robust pattern for resource management. ```go tracer := otel.GetTracer(ctx, appName) ctx, span := tracer.Start(ctx, appName+".startup", trace.WithAttributes( attribute.Int("appServices", len(appServices)), )) defer span.End() // Ensure the span is always ended ``` * **File:** main.go * **Line:** 22 * **Issue:** The `appName` constant is hardcoded in `main.go` and then updated via a `sed` command in the `Makefile`'s `rename` target. This creates a brittle coupling between the Go source code and the build script. If the `sed` pattern changes or the constant name changes, the `Makefile` might fail to update it, leading to inconsistencies. * **Suggestion:** For values that need to be configurable at build time (like an application name), consider using Go's `ldflags` to inject the value during compilation. This decouples the source code from the build script's `sed` operations and makes the build more robust. * Example: In `main.go`: `var appName = "demo-app"` (or `var appName string`) In `Makefile`: `go build -ldflags "-X main.appName=$(APP)"`
rmcguire added 2 commits 2025-08-14 19:59:18 +00:00
refactors air.toml configuration, updates Go dependencies, and improves application startup telemetry and signal handling. 925c3f34a9
update gitignore 406d5fe5cf
rmcguire merged commit ff2e489c97 into main 2025-08-14 20:00:14 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
rmcguire
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rmcguire/go-server-with-otel#3
No description provided.
Delete Branch "development"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?