Skip to content

Commit

Permalink
Merge in master to grpc rebase (#575)
Browse files Browse the repository at this point in the history
* Upgrade thrift to 0.17.0

This allows us to remove tDuplicateToProtocol and getClientError
implementation as they are in thrift v0.17.0 now.

There's no breaking changes between 0.16.0 and 0.17.0 regarding go
library/compiler.

* breakerbp: Fix prometheus gauge

In the current implementation, the gauge value is only set when the
state of the breaker changes. Which means if a breaker never got
tripped, we would not report the gauge value at all.

Set an initial value of 1 in constructor.

* Update gRPC metrics to match v2 and spec (#572)

* feat(secrets): support for csi directories (#565)

Co-authored-by: Ted Dorfeuille <ted.dorfeuille@reddit.com>
Co-authored-by: Matt Terwilliger <matt@terwilligers.com>

Co-authored-by: Yuxuan 'fishy' Wang <yuxuan.wang@reddit.com>
Co-authored-by: Kyle Lemons <kyle.lemons@reddit.com>
Co-authored-by: Ted <Tediferous@users.noreply.github.com>
Co-authored-by: Ted Dorfeuille <ted.dorfeuille@reddit.com>
Co-authored-by: Matt Terwilliger <matt@terwilligers.com>
  • Loading branch information
6 people committed Oct 25, 2022
1 parent adcee7f commit 32e40d7
Show file tree
Hide file tree
Showing 20 changed files with 280 additions and 441 deletions.
10 changes: 4 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,16 @@ See [here](Style.md).

## Thrift generated files

The `internal/gen-go/` directory contains thrift generated files,
with `*-remote` directories removed.
The `internal/gen-go/` directory contains thrift generated files.
They are excluded from the linter.
DO NOT EDIT.

They were generated with [thrift compiler v0.16.0][thrift-version] against
They were generated with [thrift compiler v0.17.0][thrift-version] against
[`baseplate.thrift`][baseplate.thrift]
using the following commands under `internal/`:

```
thrift --gen go:package_prefix=github.com/reddit/baseplate.go/ path/to/baseplate.thrift
find gen-go -depth -name "*-remote" -type d -exec rm -Rf {} \;
thrift --gen go:package_prefix=github.com/reddit/baseplate.go/,skip_remote path/to/baseplate.thrift
```

They are needed by some of the Baseplate.go packages.
Expand All @@ -41,4 +39,4 @@ files changed significantly.

[godev]: https://pkg.go.dev/github.com/reddit/baseplate.go

[thrift-version]: https://github.com/apache/thrift/releases/tag/v0.16.0
[thrift-version]: https://github.com/apache/thrift/releases/tag/v0.17.0
5 changes: 5 additions & 0 deletions breakerbp/failure_ratio.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ func NewFailureRatioBreaker(config Config) FailureRatioBreaker {
if config.EmitStatusMetrics {
go failureBreaker.runStatsProducer(config.EmitStatusMetricsInterval)
}

breakerClosed.With(prometheus.Labels{
nameLabel: config.Name,
}).Set(1)

return failureBreaker
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.18
require (
github.com/Shopify/sarama v1.29.1
github.com/alicebob/miniredis/v2 v2.14.3
github.com/apache/thrift v0.16.0
github.com/apache/thrift v0.17.0
github.com/avast/retry-go v3.0.0+incompatible
github.com/fsnotify/fsnotify v1.5.4
github.com/getsentry/sentry-go v0.11.0
Expand Down
5 changes: 2 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a/go.mod h1:SGn
github.com/alicebob/miniredis/v2 v2.14.3 h1:QWoo2wchYmLgOB6ctlTt2dewQ1Vu6phl+iQbwT8SYGo=
github.com/alicebob/miniredis/v2 v2.14.3/go.mod h1:gquAfGbzn92jvtrSC69+6zZnwSODVXVpYDRaGhWaL6I=
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
github.com/apache/thrift v0.16.0 h1:qEy6UW60iVOlUy+b9ZR0d5WzUWYGOo4HfopoyBaNmoY=
github.com/apache/thrift v0.16.0/go.mod h1:PHK3hniurgQaNMZYaCLEqXKsYK8upmhPbmdP2FXSqgU=
github.com/apache/thrift v0.17.0 h1:cMd2aj52n+8VoAtvSvLn4kDC3aZ6IAkBuqWQ2IDu7wo=
github.com/apache/thrift v0.17.0/go.mod h1:OLxhMRJxomX+1I/KUw03qoV3mMz16BwaKI+d4fPBx7Q=
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
github.com/avast/retry-go v3.0.0+incompatible h1:4SOWQ7Qs+oroOTQOYnAHqelpCO0biHSxpiH9JdtuBj0=
github.com/avast/retry-go v3.0.0+incompatible/go.mod h1:XtSnn+n/sHqQIpZ10K1qAevBhOOCWBLXXy3hyiqqBrY=
Expand Down Expand Up @@ -165,7 +165,6 @@ github.com/golang/mock v1.4.0/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt
github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4=
github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
Expand Down
1 change: 1 addition & 0 deletions grpcbp/client_middlewares.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ func PrometheusUnaryClientInterceptor(serverSlug string) grpc.UnaryClientInterce
activeRequestLabels := prometheus.Labels{
serviceLabel: serviceName,
methodLabel: method,
typeLabel: unary,
clientNameLabel: serverSlug,
}
clientActiveRequests.With(activeRequestLabels).Inc()
Expand Down
2 changes: 2 additions & 0 deletions grpcbp/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ var (
serverActiveRequestsLabels = []string{
serviceLabel,
methodLabel,
typeLabel,
}

serverActiveRequests = promauto.With(prometheusbpint.GlobalRegistry).NewGaugeVec(prometheus.GaugeOpts{
Expand Down Expand Up @@ -95,6 +96,7 @@ var (
clientActiveRequestsLabels = []string{
serviceLabel,
methodLabel,
typeLabel,
clientNameLabel,
}

Expand Down
2 changes: 2 additions & 0 deletions grpcbp/server_middlewares.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"google.golang.org/grpc/status"

"github.com/prometheus/client_golang/prometheus"

"github.com/reddit/baseplate.go/ecinterface"
"github.com/reddit/baseplate.go/log"
"github.com/reddit/baseplate.go/prometheusbp"
Expand Down Expand Up @@ -198,6 +199,7 @@ func InjectPrometheusUnaryServerInterceptor() grpc.UnaryServerInterceptor {

activeRequestLabels := prometheus.Labels{
serviceLabel: serviceName,
typeLabel: unary,
methodLabel: method,
}
serverActiveRequests.With(activeRequestLabels).Inc()
Expand Down
2 changes: 2 additions & 0 deletions grpcbp/server_middlewares_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ func TestInjectPrometheusUnaryServerClientInterceptor(t *testing.T) {

serverActiveRequestLabels := prometheus.Labels{
serviceLabel: serviceName,
typeLabel: unary,
methodLabel: tt.method,
}

Expand All @@ -277,6 +278,7 @@ func TestInjectPrometheusUnaryServerClientInterceptor(t *testing.T) {
clientActiveRequestLabels := prometheus.Labels{
serviceLabel: serviceName,
methodLabel: tt.method,
typeLabel: unary,
clientNameLabel: serverSlug,
}

Expand Down
2 changes: 1 addition & 1 deletion internal/gen-go/reddit/baseplate/GoUnusedProtection__.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion internal/gen-go/reddit/baseplate/baseplate-consts.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

135 changes: 79 additions & 56 deletions internal/gen-go/reddit/baseplate/baseplate.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions internal/prometheusbpint/spectest/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,12 @@ func thriftSpecificLabels(name string) []string {
// - "service"
// - "method"
func grpcSpecificLabels(name string) []string {
labelSuffixes := []string{"service", "method"}
labelSuffixes := []string{"service", "method", "type"}
switch {
case strings.HasSuffix(name, "_latency_seconds"):
labelSuffixes = append(labelSuffixes, "type", "success")
labelSuffixes = append(labelSuffixes, "success")
case strings.HasSuffix(name, "_requests_total"):
labelSuffixes = append(labelSuffixes, "type", "success", "code")
labelSuffixes = append(labelSuffixes, "success", "code")
case strings.HasSuffix(name, "_active_requests"):
// no op
default:
Expand Down
1 change: 1 addition & 0 deletions internal/prometheusbpint/spectest/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ func TestBuildLabelsGPRC(t *testing.T) {
want: map[string]struct{}{
"grpc_method": {},
"grpc_service": {},
"grpc_type": {},
},
},
{
Expand Down
Loading

0 comments on commit 32e40d7

Please sign in to comment.