Skip to content
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Improve performance by reducing allocations in the gRPC stats handler in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#8035)

### Fixed

- Fix issue in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin` middleware where temporary files for multipart/form-data were not being removed. (#6609)

### Deprecated

- `WithRouteTag` in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` is deprecated.
Expand Down
6 changes: 5 additions & 1 deletion instrumentation/github.com/gin-gonic/gin/otelgin/gin.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,11 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
c.Set(tracerKey, tracer)
savedCtx := c.Request.Context()
defer func() {
c.Request = c.Request.WithContext(savedCtx)
if c.Request.MultipartForm != nil {
c.Request = c.Request.Clone(savedCtx)
} else {
c.Request = c.Request.WithContext(savedCtx)
}
}()
ctx := cfg.Propagators.Extract(savedCtx, propagation.HeaderCarrier(c.Request.Header))

Expand Down
90 changes: 90 additions & 0 deletions instrumentation/github.com/gin-gonic/gin/otelgin/gin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@
package otelgin_test

import (
"bytes"
"errors"
"fmt"
"html/template"
"io/fs"
"mime/multipart"
"net/http"
"net/http/httptest"
"os"
"runtime"
"strconv"
"strings"
"testing"
Expand Down Expand Up @@ -479,6 +484,91 @@
})
}

func TestTemporaryFormFileRemove(t *testing.T) {
if runtime.GOOS == "windows" {
// Windows sometimes refuses to remove a file that was just closed.
t.Skip("https://go.dev/issue/25965")
}
sr := tracetest.NewSpanRecorder()
provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sr))

router := gin.New()
router.MaxMultipartMemory = 1

tempDir := t.TempDir()

// We register three middlewares here, with the otel one in the middle.
// When the response is returned, the post-handler execution order is:
// MiddlewareC -> otel -> MiddlewareA (reverse of registration order).
// In MiddlewareC, the multipart form file still exists, so opening it succeeds.
// However, by the time MiddlewareA runs, the otel middleware has already
// removed the temporary multipart files, so opening the file results in a
// "file does not exist" error.

// MiddlewareA
router.Use(func(c *gin.Context) {
c.Next()
_, err := c.FormFile("files")
require.NoError(t, err)
form, _ := c.MultipartForm()
files := form.File["files"]
require.Len(t, files, 1)
_, err = fs.ReadDir(os.DirFS(tempDir), files[0].Filename)
require.Error(t, err)
require.True(t, errors.Is(err, fs.ErrNotExist))

Check failure on line 518 in instrumentation/github.com/gin-gonic/gin/otelgin/gin_test.go

View workflow job for this annotation

GitHub Actions / lint

error-is-as: use require.ErrorIs (testifylint)
})

router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider(provider)))

// MiddlewareC
router.Use(func(c *gin.Context) {
c.Next()
_, err := c.FormFile("files")
require.NoError(t, err)
form, _ := c.MultipartForm()
files := form.File["files"]
require.Len(t, files, 1)
_, err = fs.ReadDir(os.DirFS(tempDir), files[0].Filename)
require.Error(t, err)
require.True(t, errors.Is(err, fs.ErrNotExist))

Check failure on line 533 in instrumentation/github.com/gin-gonic/gin/otelgin/gin_test.go

View workflow job for this annotation

GitHub Actions / lint

error-is-as: use require.ErrorIs (testifylint)
})

var fileHeader *multipart.FileHeader
router.POST("/upload", func(c *gin.Context) {
_, err := c.FormFile("files")
require.NoError(t, err)
fileHeader = c.Request.MultipartForm.File["files"][0]
_, err = fileHeader.Open()
require.NoError(t, err)
c.JSON(http.StatusOK, nil)
})

var body bytes.Buffer

mw := multipart.NewWriter(&body)
fw, err := mw.CreateFormFile("files", "file")
require.NoError(t, err)

_, err = fw.Write([]byte("hello world"))
require.NoError(t, err)
err = mw.Close()
require.NoError(t, err)
r := httptest.NewRequest("POST", "/upload", &body)
r.Header.Add("Content-Type", mw.FormDataContentType())
w := httptest.NewRecorder()

router.ServeHTTP(w, r)
assert.Len(t, sr.Ended(), 1)
require.Equal(t, http.StatusOK, w.Code)
_, err = fileHeader.Open()
require.NoError(t, err)

// check TempDir is empty
entries, err := os.ReadDir(tempDir)
require.NoError(t, err)
require.Len(t, entries, 0)

Check failure on line 569 in instrumentation/github.com/gin-gonic/gin/otelgin/gin_test.go

View workflow job for this annotation

GitHub Actions / lint

empty: use require.Empty (testifylint)
}

func TestMetrics(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading