Update go-grpc-middleware to v2.0.0 #6651

coleenquadros · 2023-08-24T12:41:13Z

Update go_grpc_middleware to v2.0.0.
Remove Tags Interceptor from Thanos. Tags interceptor is removed from v2.0.0 go-grpc-middleware.
Tracing interceptor directory from go_grpc_middleware(v2.0.0 rc 2,3,4) moved to Thanos(v2.0.0 makes use of otel). Tracing interceptor was removed in version v2.0.0. The follow up in Thanos would be to migrate to otel.
Logging middleware migrated to use v2.0.0 logging interceptors. Trace ID is now logged as a part of GRPC request logging(output below).
(PR for adding trace ID to HTTP logging middleware - add trace id to http logging middleware #6611)

ts=2023-10-20T11:34:12.98939Z caller=grpc.go:107 level=debug name=store service=gRPC/server component=store msg="started call" traceID=5ee8eb9e1b05437670f2d311252aa755 requestID=01HD6DTJQXHWXEHBS7YE7QVJ22 protocol=grpc grpc.component=server grpc.service=thanos.info.Info grpc.method=Info grpc.method_type=unary peer.address=127.0.0.1:52802 grpc.start_time=2023-10-20T13:34:12+02:00 grpc.request.deadline=2023-10-20T13:34:17+02:00 grpc.time_ms=0.145
ts=2023-10-20T11:34:12.989388Z caller=grpc.go:107 level=debug name=store service=gRPC/server component=store msg="started call" traceID=d25ad23a02c034c84fc856a4d6a59fdb requestID=01HD6DTJQX368WC3KT5R45H4WY protocol=grpc grpc.component=server grpc.service=thanos.info.Info grpc.method=Info grpc.method_type=unary peer.address=127.0.0.1:52801 grpc.start_time=2023-10-20T13:34:12+02:00 grpc.request.deadline=2023-10-20T13:34:17+02:00 grpc.time_ms=0.137
ts=2023-10-20T11:34:12.989441Z caller=grpc.go:107 level=debug name=store service=gRPC/server component=store msg="finished call" traceID=5ee8eb9e1b05437670f2d311252aa755 requestID=01HD6DTJQXHWXEHBS7YE7QVJ22 protocol=grpc grpc.component=server grpc.service=thanos.info.Info grpc.method=Info grpc.method_type=unary peer.address=127.0.0.1:52802 grpc.start_time=2023-10-20T13:34:12+02:00 grpc.request.deadline=2023-10-20T13:34:17+02:00 grpc.code=OK grpc.time_ms=0.2
ts=2023-10-20T11:34:12.989445Z caller=grpc.go:107 level=debug name=store service=gRPC/server component=store msg="finished call" traceID=d25ad23a02c034c84fc856a4d6a59fdb requestID=01HD6DTJQX368WC3KT5R45H4WY protocol=grpc grpc.component=server grpc.service=thanos.info.Info grpc.method=Info grpc.method_type=unary peer.address=127.0.0.1:52801 grpc.start_time=2023-10-20T13:34:12+02:00 grpc.request.deadline=2023-10-20T13:34:17+02:00 grpc.code=OK grpc.time_ms=0.199

I added CHANGELOG entry for this change.
Change is not relevant to the end user.

Changes

Verification

moadz

LGTM, thanks for this :) Please fix linting failures. I believe the e2e failure is a flake.

moadz · 2023-09-27T08:38:19Z

go.mod

 	github.com/dgryski/go-metro v0.0.0-20200812162917-85c65e2d0165 // indirect
 	github.com/golang-jwt/jwt/v5 v5.0.0 // indirect
 	github.com/google/s2a-go v0.1.4 // indirect
+	github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 // indirect


What's the source of the 1.3.0 import from?

It might be some dependency that uses old grpc-middleware (likely)

pkg/logging/grpc.go

pkg/logging/options.go

moadz · 2023-09-27T09:27:26Z

pkg/tracing/interceptors/reporter.go

@@ -0,0 +1,82 @@
+// Copyright 2016 Michal Witkowski. All Rights Reserved.


Just for clarification:

We are doing this temporarily until we migrate Thanos Tracing to OTEL provider, instead of OpenTracing. I do believe most of them are OTEL compatible now, but it was a good idea to split it up.

moadz · 2023-09-27T09:29:43Z

@yeya24, @matej-g any chance we could get a review to try to unblock go-grpc-middleware upgrade?

douglascamata

Some comments from a 1st pass

CONTRIBUTING.md

CHANGELOG.md

pkg/logging/grpc.go

pkg/tracing/tracing_middleware/server.go

pkg/tracing/interceptors/client.go

coleenquadros · 2023-10-02T14:09:32Z

pkg/tracing/interceptors/reporter.go

+/*
+This was copied over from https://github.com/grpc-ecosystem/go-grpc-middleware/tree/v2.0.0-rc.3
+and modified to support tracing in Thanos till migration to Otel is supported.
+*/


Since this directory was copied from go-grpc-middleware repo. I was not sure how to handle the licensing.

douglascamata

LGTM ✅

CHANGELOG.md

yeya24 · 2023-10-17T06:16:28Z

It would be great if we can move this forward. I am kind of blocked by this right now.

yeya24

LGTM. I tried this branch locally and fixed conflicts. It worked for me.
I didn't test all the features changed though, like logging and tracing.

fpetkovski · 2023-10-18T13:49:55Z

Looks like unit tests are still failing.

douglascamata · 2023-11-20T10:23:38Z

Could we get another round of reviews, please? 🥺

fpetkovski · 2024-05-03T07:04:08Z

@coleenquadros would you mind rebasing and we can merge this PR.

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

…grpc middleware Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

coleenquadros · 2024-05-27T11:25:09Z

@fpetkovski @pedro-stanaka I have rebased the changes. Sorry for the delay in response.

pedro-stanaka

Thx for your work, this will unblock a lot of nice improvements around metric instrumentation for thanos.

fpetkovski

Let's merge before we run into another conflict.

pull-request-size bot added the size/L label Aug 24, 2023

coleenquadros changed the title ~~remove tags interceptor~~ Remove tags options in Thanos grpc logging middleware Aug 24, 2023

coleenquadros marked this pull request as ready for review August 24, 2023 14:55

coleenquadros marked this pull request as draft August 31, 2023 15:33

coleenquadros force-pushed the update_grpc_mw branch 3 times, most recently from 861b8cf to a66c22e Compare August 31, 2023 16:56

pull-request-size bot added size/M and removed size/L labels Aug 31, 2023

coleenquadros force-pushed the update_grpc_mw branch from 1f7d10b to 2c1ffdf Compare August 31, 2023 16:59

coleenquadros marked this pull request as ready for review August 31, 2023 17:10

pull-request-size bot added size/XXL and removed size/M labels Sep 4, 2023

coleenquadros force-pushed the update_grpc_mw branch from 4a463e6 to 71b0a8f Compare September 4, 2023 11:55

coleenquadros changed the title ~~Remove tags options in Thanos grpc logging middleware~~ Update grpc logging middleware to v2.0.0 Sep 4, 2023

coleenquadros changed the title ~~Update grpc logging middleware to v2.0.0~~ Update go-grpc-middleware to v2.0.0 Sep 4, 2023

moadz approved these changes Sep 27, 2023

View reviewed changes

douglascamata reviewed Sep 27, 2023

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

pkg/logging/grpc.go Outdated Show resolved Hide resolved

pkg/tracing/tracing_middleware/server.go Outdated Show resolved Hide resolved

coleenquadros force-pushed the update_grpc_mw branch 2 times, most recently from c56eba3 to f0792ac Compare September 30, 2023 11:55

yeya24 reviewed Oct 1, 2023

View reviewed changes

pkg/tracing/interceptors/client.go Show resolved Hide resolved

coleenquadros commented Oct 2, 2023

View reviewed changes

douglascamata approved these changes Oct 3, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

yeya24 previously approved these changes Oct 17, 2023

View reviewed changes

coleenquadros dismissed yeya24’s stale review via ae8e6ed October 18, 2023 12:28

coleenquadros force-pushed the update_grpc_mw branch 2 times, most recently from a256931 to 3cd80ca Compare October 18, 2023 13:10

coleenquadros force-pushed the update_grpc_mw branch from 19cd9f6 to bfb0b26 Compare November 13, 2023 13:38

coleenquadros force-pushed the update_grpc_mw branch from bfb0b26 to 4d7e417 Compare December 14, 2023 10:12

fpetkovski previously approved these changes Apr 23, 2024

View reviewed changes

coleenquadros added 18 commits May 27, 2024 10:07

remove tags interceptor

3ab833e

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

remove tags interceptor

e01e4ab

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

add changelog

44e31e3

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

Update CHANGELOG.md

4ae6d73

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

update changelog

e540872

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

Update go_grpc_middleware to v2.0.0

7554405

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

ctx

695912e

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

Update CHANGELOG.md

42cc9c8

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

add changelog

3d7da10

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

Update CHANGELOG.md

98ce7e2

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

fix taggingsuite test

789ef6a

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

lint

61fc465

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

add request id in logging field

95d4c57

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

lint

2789d1b

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

docs-s

7849529

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

lint

3e863ef

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

linting

bd1f066

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

changelog

b84242a

Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

coleenquadros dismissed fpetkovski’s stale review via b84242a May 27, 2024 10:22

coleenquadros force-pushed the update_grpc_mw branch from 4d971b5 to b84242a Compare May 27, 2024 10:22

update query pkg to use internal tracing pkg to accomodate update of …

cf472a9

…grpc middleware Signed-off-by: Coleen Iona Quadros <coleen.quadros27@gmail.com>

pedro-stanaka approved these changes May 27, 2024

View reviewed changes

fpetkovski approved these changes May 27, 2024

View reviewed changes

fpetkovski merged commit c03e70f into thanos-io:main May 27, 2024
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update go-grpc-middleware to v2.0.0 #6651

Update go-grpc-middleware to v2.0.0 #6651

coleenquadros commented Aug 24, 2023 •

edited

Loading

moadz left a comment •

edited

Loading

moadz Sep 27, 2023

bwplotka Oct 3, 2023

moadz Sep 27, 2023

moadz commented Sep 27, 2023

douglascamata left a comment

coleenquadros Oct 2, 2023

douglascamata left a comment

yeya24 commented Oct 17, 2023

yeya24 left a comment •

edited

Loading

fpetkovski commented Oct 18, 2023

douglascamata commented Nov 20, 2023

fpetkovski commented May 3, 2024

coleenquadros commented May 27, 2024

pedro-stanaka left a comment

fpetkovski left a comment •

edited

Loading

		@@ -0,0 +1,82 @@
		// Copyright 2016 Michal Witkowski. All Rights Reserved.

Update go-grpc-middleware to v2.0.0 #6651

Update go-grpc-middleware to v2.0.0 #6651

Conversation

coleenquadros commented Aug 24, 2023 • edited Loading

Changes

Verification

moadz left a comment • edited Loading

Choose a reason for hiding this comment

moadz Sep 27, 2023

Choose a reason for hiding this comment

bwplotka Oct 3, 2023

Choose a reason for hiding this comment

moadz Sep 27, 2023

Choose a reason for hiding this comment

moadz commented Sep 27, 2023

douglascamata left a comment

Choose a reason for hiding this comment

coleenquadros Oct 2, 2023

Choose a reason for hiding this comment

douglascamata left a comment

Choose a reason for hiding this comment

yeya24 commented Oct 17, 2023

yeya24 left a comment • edited Loading

Choose a reason for hiding this comment

fpetkovski commented Oct 18, 2023

douglascamata commented Nov 20, 2023

fpetkovski commented May 3, 2024

coleenquadros commented May 27, 2024

pedro-stanaka left a comment

Choose a reason for hiding this comment

fpetkovski left a comment • edited Loading

Choose a reason for hiding this comment

coleenquadros commented Aug 24, 2023 •

edited

Loading

moadz left a comment •

edited

Loading

yeya24 left a comment •

edited

Loading

fpetkovski left a comment •

edited

Loading