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

ghttp middleware tracing need ungzip response when header contains gzip #2792

Closed
ltp217 opened this issue Jul 21, 2023 · 1 comment
Closed
Labels
enhancement help wanted planned This issue/proposal is planned into our next steps.

Comments

@ltp217
Copy link
Contributor

ltp217 commented Jul 21, 2023

1. What version of Go and system type/arch are you using?

go 1.19, linux/amd64

2. What version of GoFrame are you using?

goframe version v2.2.2

3. Can this issue be re-produced with the latest release?

yes, can re-produced in the latest release, v2.5.0

4. What did you do?

when use goframe server, handle the /metrics to send trace infomation to the otel-collector, it will change to grpc protocol,but will get 'invalid utf-8' error.
the reason is that, when handle to '/metrics', if not set DisableCompress to true, and the header in the request contains 'Account-Encoding: gzip', the response will be gziped. you can see the code in :
https://github.com/prometheus/client_golang/blob/main/prometheus/promhttp/http.go#L172.
And in goframe server middleware, it will set the attribute "http.response.body" with the value of response, but now the response is now gziped, it will not be valid utf-8 string.
So when send traces by grpc in https://github.com/protocolbuffers/protobuf-go/blob/master/internal/impl/codec_gen.go#L5028, it will return "invalid utf-8" error.
image
image
prometheus server like this:

func New(ctx context.Context, opts ...Opt) *ghttp.Server {
	config, err := initConfig(ctx, opts...)
	if err != nil {
		log.New().Fatalf(ctx, "init config fail, err:%+v", err)
	}
	s := g.Server(defaultServerName)
	s.SetAddr(config.Address)
	s.SetDumpRouterMap(false)
	// metrics采集不需要swagger接口
	s.SetSwaggerPath("")
	s.SetOpenApiPath("")
	s.BindHandler("/metrics", func(r *ghttp.Request) {
		promhttp.InstrumentMetricHandler(
			prometheus.DefaultRegisterer,
			promhttp.HandlerFor(prometheus.DefaultGatherer,
				promhttp.HandlerOpts{})).ServeHTTP(r.Response.Writer, r.Request)
	})
	return s
}

before the promhttp.HandlerOpts don't contains 'DisableCompression: true'
and use the otel by grpc protocol, can see:https://github.com/open-telemetry/opentelemetry-go/tree/main/example/otel-collector.

5. What did you expect to see?

maybe in goframe, when add attribute in the server middleware, need to check the header contains 'gzip' or not , if contains, the response should be ungzip, and then set the ungziped string to the attribute.

@gqcn gqcn added enhancement help wanted planned This issue/proposal is planned into our next steps. labels Sep 25, 2023
@github-actions
Copy link

Hello @ltp217. We like your proposal/feedback and would appreciate a contribution via a Pull Request by you or another community member. We thank you in advance for your contribution and are looking forward to reviewing it!
你好 @ltp217。我们喜欢您的提案/反馈,并希望您或其他社区成员通过拉取请求做出贡献。我们提前感谢您的贡献,并期待对其进行审查。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted planned This issue/proposal is planned into our next steps.
Projects
None yet
Development

No branches or pull requests

3 participants