Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using otelgin.Middleware will cause temporary MultipartForm files not to be destroyed. #5946

Open
zacklpx opened this issue Jul 25, 2024 · 3 comments · May be fixed by #6609
Open

Using otelgin.Middleware will cause temporary MultipartForm files not to be destroyed. #5946

zacklpx opened this issue Jul 25, 2024 · 3 comments · May be fixed by #6609
Labels
area: instrumentation Related to an instrumentation package bug Something isn't working instrumentation: otelgin

Comments

@zacklpx
Copy link

zacklpx commented Jul 25, 2024

Description

Using otelgin.Middleware will cause temporary MultipartForm files not to be destroyed.

Environment

  • OS: MacOS
  • Architecture: x86
  • Go Version: 1.22
  • otelgin version: 0.53.0

Steps To Reproduce

  1. Using this code start up gin http server
package main

import (
	"net/http"

	"github.com/gin-gonic/gin"
	"go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin"
)

func main() {
	engine := gin.Default()
	engine.MaxMultipartMemory = 1024 * 1024
	engine.Use(otelgin.Middleware("my-server"))

	engine.POST("/upload", func(c *gin.Context) {
		ff, err := c.FormFile("file")
		if err != nil {
			_ = c.AbortWithError(http.StatusInternalServerError, err)
			return
		}
		_ = ff
		c.JSON(http.StatusOK, map[string]interface{}{"c": 0})
	})

	engine.Run(":8624")
}
  1. Upload a file to the http api
curl --request POST \
  --url http://127.0.0.1:8624/upload \
  --header 'Content-Type: multipart/form-data' \
  --header 'User-Agent: insomnia/9.3.2' \
  --form '[email protected]'

Expected behavior

Upload a file larger than MaxMultipartMemory, and the HTTP server will generate multipart-xxxx temporary files in the system temporary directory. These files will be automatically destroyed when the request ends. However, if the otelgin.Middleware is used, the temporary files will not be destroyed.

the HTTP server holds the original request, and using otelgin.Middleware replaces the request with c.request = c.request.WithContext(ctx), causing subsequent c.FormFile reads to fall on the new request. As a result, the original request held by the HTTP server does not have the MultipartForm, preventing it from being destroyed.

@zacklpx zacklpx added area: instrumentation Related to an instrumentation package bug Something isn't working instrumentation: otelgin labels Jul 25, 2024
@Yiling-J
Copy link

@akats7 Is there any plan to address this issue? For reference, a similar problem was resolved in Echo with this fix: Echo-contrib PR #90. If you're busy, I'd be happy to help by creating a PR to address it. Let me know!

@dmathieu
Copy link
Member

cc @akats7 as code owner.

@akats7
Copy link
Contributor

akats7 commented Jan 13, 2025

Hi @Yiling-J,

Feel free to open a PR, I'll gladly review

@Yiling-J Yiling-J linked a pull request Jan 13, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: instrumentation Related to an instrumentation package bug Something isn't working instrumentation: otelgin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants