Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions examples/dice/instrumented/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,12 @@ func run() error {
}()

// Start HTTP server.
port := os.Getenv("APPLICATION_PORT")
if port == "" {
port = "8080"
}
srv := &http.Server{
Addr: ":8080",
Addr: ":" + port,
BaseContext: func(net.Listener) context.Context { return ctx },
ReadTimeout: time.Second,
WriteTimeout: 10 * time.Second,
Expand Down Expand Up @@ -72,8 +76,7 @@ func newHTTPHandler() http.Handler {
mux := http.NewServeMux()

// Register handlers.
mux.Handle("/rolldice", http.HandlerFunc(rolldice))
mux.Handle("/rolldice/{player}", http.HandlerFunc(rolldice))
handleFunc("/rolldice", handleRollDice)

// Add HTTP instrumentation for the whole server.
handler := otelhttp.NewHandler(mux, "/")
Expand Down
34 changes: 20 additions & 14 deletions examples/dice/instrumented/otel.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/sdk/log"
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/resource"
"go.opentelemetry.io/otel/sdk/trace"
)

Expand All @@ -42,12 +43,19 @@ func setupOTelSDK(ctx context.Context) (func(context.Context) error, error) {
err = errors.Join(inErr, shutdown(ctx))
}

res, _ := resource.New(ctx,
resource.WithFromEnv(), // reads OTEL_SERVICE_NAME & OTEL_RESOURCE_ATTRIBUTES.
resource.WithProcess(),
resource.WithHost(),
resource.WithTelemetrySDK(),
)
Comment on lines +46 to +51
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error returned by resource.New() is being silently ignored. This could lead to using a nil or incomplete resource, potentially causing issues with telemetry data. Either handle the error properly or at minimum log it if you intend to continue with a partial resource.

Suggested change
res, _ := resource.New(ctx,
resource.WithFromEnv(), // reads OTEL_SERVICE_NAME & OTEL_RESOURCE_ATTRIBUTES.
resource.WithProcess(),
resource.WithHost(),
resource.WithTelemetrySDK(),
)
res, resErr := resource.New(ctx,
resource.WithFromEnv(), // reads OTEL_SERVICE_NAME & OTEL_RESOURCE_ATTRIBUTES.
resource.WithProcess(),
resource.WithHost(),
resource.WithTelemetrySDK(),
)
if resErr != nil {
handleErr(resErr)
return shutdown, resErr
}

Copilot uses AI. Check for mistakes.

// Set up propagator.
prop := newPropagator()
otel.SetTextMapPropagator(prop)

// Set up trace provider.
tracerProvider, err := newtracerProvider()
tracerProvider, err := newtracerProvider(res)
if err != nil {
handleErr(err)
return shutdown, err
Expand All @@ -56,7 +64,7 @@ func setupOTelSDK(ctx context.Context) (func(context.Context) error, error) {
otel.SetTracerProvider(tracerProvider)

// Set up meter provider.
meterProvider, err := newMeterProvider()
meterProvider, err := newMeterProvider(res)
if err != nil {
handleErr(err)
return shutdown, err
Expand All @@ -65,7 +73,7 @@ func setupOTelSDK(ctx context.Context) (func(context.Context) error, error) {
otel.SetMeterProvider(meterProvider)

// Set up logger provider.
loggerProvider, err := newLoggerProvider()
loggerProvider, err := newLoggerProvider(res)
if err != nil {
handleErr(err)
return shutdown, err
Expand All @@ -83,43 +91,41 @@ func newPropagator() propagation.TextMapPropagator {
)
}

func newtracerProvider() (*trace.TracerProvider, error) {
traceExporter, err := stdouttrace.New(
stdouttrace.WithPrettyPrint())
func newtracerProvider(res *resource.Resource) (*trace.TracerProvider, error) {
traceExporter, err := stdouttrace.New(stdouttrace.WithPrettyPrint())
if err != nil {
return nil, err
}

tracerProvider := trace.NewTracerProvider(
trace.WithBatcher(traceExporter,
// Default is 5s. Set to 1s for demonstrative purposes.
trace.WithBatchTimeout(time.Second)),
trace.WithBatcher(traceExporter, trace.WithBatchTimeout(time.Second)), // Default is 5s. Set to 1s for demonstrative purposes.
trace.WithResource(res),
)
return tracerProvider, nil
}

func newMeterProvider() (*metric.MeterProvider, error) {
func newMeterProvider(res *resource.Resource) (*metric.MeterProvider, error) {
metricExporter, err := stdoutmetric.New()
if err != nil {
return nil, err
}

meterProvider := metric.NewMeterProvider(
metric.WithReader(metric.NewPeriodicReader(metricExporter,
// Default is 1m. Set to 3s for demonstrative purposes.
metric.WithInterval(3*time.Second))),
metric.WithReader(metric.NewPeriodicReader(metricExporter, metric.WithInterval(3*time.Second))), // Default is 1m. Set to 3s for demonstrative purposes.
metric.WithResource(res),
)
return meterProvider, nil
}

func newLoggerProvider() (*log.LoggerProvider, error) {
func newLoggerProvider(res *resource.Resource) (*log.LoggerProvider, error) {
logExporter, err := stdoutlog.New()
if err != nil {
return nil, err
}

loggerProvider := log.NewLoggerProvider(
log.WithProcessor(log.NewBatchProcessor(logExporter)),
log.WithResource(res),
)
return loggerProvider, nil
}
142 changes: 122 additions & 20 deletions examples/dice/instrumented/rolldice.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
package main

import (
"io"
"context"
"encoding/json"
"errors"
"log"
"math/rand"
"net/http"
"strconv"
Expand All @@ -19,42 +22,141 @@ import (
const name = "go.opentelemetry.io/contrib/examples/dice"

var (
tracer = otel.Tracer(name)
meter = otel.Meter(name)
logger = otelslog.NewLogger(name)
rollCnt metric.Int64Counter
tracer = otel.Tracer(name)
meter = otel.Meter(name)
logger = otelslog.NewLogger(name)
rollCnt metric.Int64Counter
outcomeHist metric.Int64Histogram
lastRollsGauge metric.Int64ObservableGauge
lastRolls int64
)

func init() {
var err error
rollCnt, err = meter.Int64Counter("dice.rolls",
metric.WithDescription("The number of rolls by roll value"),
metric.WithDescription("The number of rolls"),
metric.WithUnit("{roll}"))
if err != nil {
panic(err)
}

outcomeHist, err = meter.Int64Histogram(
"dice.outcome",
metric.WithDescription("Distribution of dice outcomes (1-6)"),
metric.WithUnit("{count}"),
)
if err != nil {
panic(err)
}

lastRollsGauge, err = meter.Int64ObservableGauge(
"dice.last.rolls",
metric.WithDescription("The last rolls value observed"),
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The description "The last rolls value observed" is unclear. Consider clarifying what "rolls value" means, e.g., "The number of dice rolled in the most recent request" to better describe what this gauge represents.

Suggested change
metric.WithDescription("The last rolls value observed"),
metric.WithDescription("The number of dice rolled in the most recent request"),

Copilot uses AI. Check for mistakes.
)
if err != nil {
panic(err)
}

// Register the gauge callback.
_, err = meter.RegisterCallback(
func(ctx context.Context, o metric.Observer) error {
o.ObserveInt64(lastRollsGauge, lastRolls)
return nil
},
lastRollsGauge,
)
if err != nil {
panic(err)
}
}

func rolldice(w http.ResponseWriter, r *http.Request) {
ctx, span := tracer.Start(r.Context(), "roll")
defer span.End()
func handleRollDice(w http.ResponseWriter, r *http.Request) {
// Parse query parameters.
rollsParam := r.URL.Query().Get("rolls")
player := r.URL.Query().Get("player")

roll := 1 + rand.Intn(6) //nolint:gosec // G404: Use of weak random number generator (math/rand instead of crypto/rand) is ignored as this is not security-sensitive.
// Default rolls = 1 if not defined.
if rollsParam == "" {
rollsParam = "1"
}

// Check if rolls is a number.
rolls, err := strconv.Atoi(rollsParam)
if err != nil {
w.WriteHeader(http.StatusBadRequest)
msg := "Parameter rolls must be a positive integer"
_ = json.NewEncoder(w).Encode(map[string]string{
Copy link
Member

@pellared pellared Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proper error handling is missing here (500 Internal Server Error).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pellared Isn't it refering to this line -
if rolls is set to an invalid input (not a number): status code 400 and {"status": "error", "message": "Parameter rolls must be a positive integer"} ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean if the json encoding fails then it is an internal server error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that the chances of static map encoding failure is very less. Can I handle it like this?

if err != nil {
    w.Header().Set("Content-Type", "application/json")
    w.WriteHeader(http.StatusBadRequest)

    msg := "Parameter rolls must be a positive integer"
    resp := map[string]string{
        "status":  "error",
        "message": msg,
    }

    if encErr := json.NewEncoder(w).Encode(resp); encErr != nil {
        logger.ErrorContext(r.Context(), "failed to write error JSON", "error", encErr)
        http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
        return
    }

    logger.WarnContext(r.Context(), msg)
    return
}

"status": "error",
"message": msg,
})
logger.WarnContext(r.Context(), msg)
return
}

results, err := rollDice(r.Context(), rolls)
if err != nil {
// Signals invalid input (<=0).
w.WriteHeader(http.StatusInternalServerError)
log.Printf("ERROR: %v", err)
return
}

var msg string
if player := r.PathValue("player"); player != "" {
msg = player + " is rolling the dice"
if player == "" {
log.Printf("DEBUG: anonymous player rolled %v", results)
} else {
msg = "Anonymous player is rolling the dice"
log.Printf("DEBUG: player=%s rolled %v", player, results)
}
log.Printf("INFO: %s %s -> 200 OK", r.Method, r.URL.String())

w.Header().Set("Content-Type", "application/json")
if len(results) == 1 {
json.NewEncoder(w).Encode(results[0])
} else {
json.NewEncoder(w).Encode(results)
}
}

// rollDice is the outer function which Does the error handling.
func rollDice(ctx context.Context, rolls int) ([]int, error) {
ctx, span := tracer.Start(ctx, "rollDice")
defer span.End()

if rolls <= 0 {
err := errors.New("rolls must be positive")
span.RecordError(err)
logger.ErrorContext(ctx, "error", "error", err)
return nil, err
}

if rolls == 1 {
val := rollOnce(ctx)
outcomeHist.Record(ctx, int64(val))
lastRolls = int64(rolls)
return []int{val}, nil
}

results := make([]int, rolls)
for i := 0; i < rolls; i++ {
results[i] = rollOnce(ctx)
outcomeHist.Record(ctx, int64(results[i]))
}
logger.InfoContext(ctx, msg, "result", roll)

rollsAttr := attribute.Int("rolls", rolls)
span.SetAttributes(rollsAttr)
rollCnt.Add(ctx, 1, metric.WithAttributes(rollsAttr))
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The counter is being incremented by 1 regardless of the number of rolls performed. This doesn't accurately represent the total number of dice rolls. Consider incrementing by the number of actual rolls: rollCnt.Add(ctx, int64(rolls), metric.WithAttributes(rollsAttr))

Suggested change
rollCnt.Add(ctx, 1, metric.WithAttributes(rollsAttr))
rollCnt.Add(ctx, int64(rolls), metric.WithAttributes(rollsAttr))

Copilot uses AI. Check for mistakes.
lastRolls = int64(rolls)
return results, nil
}

// rollOnce is the inner function — returns a random number 1–6.
func rollOnce(ctx context.Context) int {
ctx, span := tracer.Start(ctx, "rollOnce")
defer span.End()

roll := 1 + rand.Intn(6) //nolint:gosec // G404: Use of weak random number generator (math/rand instead of crypto/rand) is ignored as this is not security-sensitive.

rollValueAttr := attribute.Int("roll.value", roll)
span.SetAttributes(rollValueAttr)
rollCnt.Add(ctx, 1, metric.WithAttributes(rollValueAttr))

resp := strconv.Itoa(roll) + "\n"
if _, err := io.WriteString(w, resp); err != nil {
logger.ErrorContext(ctx, "Write failed", "error", err)
}
return roll
}
11 changes: 7 additions & 4 deletions examples/dice/uninstrumented/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,13 @@ func run() (err error) {
defer stop()

// Start HTTP server.
port := os.Getenv("APPLICATION_PORT")
if port == "" {
port = "8080"
}

srv := &http.Server{
Addr: ":8080",
Addr: ":" + port,
BaseContext: func(net.Listener) context.Context { return ctx },
ReadTimeout: time.Second,
WriteTimeout: 10 * time.Second,
Expand Down Expand Up @@ -60,8 +65,6 @@ func newHTTPHandler() http.Handler {
mux := http.NewServeMux()

// Register handlers.
mux.HandleFunc("/rolldice/", rolldice)
mux.HandleFunc("/rolldice/{player}", rolldice)

mux.HandleFunc("/rolldice", handleRollDice)
return mux
}
Loading