Skip to content

Commit

Permalink
New observability + general refactor of http/grpc clients (cs3org#4166)
Browse files Browse the repository at this point in the history
* remove unused fs and tracing

* add helloworld example and fix auth logic to bail out early

* remove hack on appctx and add traceid

* configurable prom collectors

* grpc reflection working

* wire http traffic

* intrument outgoing grpc calls

* add stuff

* merge appctx and ctx

* refactor rhttp to httpclient

* tidy go.mod

* remove drone tests

* add changelog

* fix test

* go 1.21

* pass tests

* fix sql tests

* go fmt

* more linters

* make all linters happy

* update docs

* add back ceph example

* remove otelhttp library

* remove files and add tests

* align with upstream
  • Loading branch information
labkode authored Oct 10, 2023
1 parent 69ac979 commit 084c1b5
Show file tree
Hide file tree
Showing 168 changed files with 3,443 additions and 1,992 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
cmd/revad/revad
cmd/revad/revad.pid
cmd/reva/reva
cmd/revad/main/main
tools/release/release

# Ignore pid files
Expand Down
6 changes: 6 additions & 0 deletions changelog/unreleased/observability.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Enhancement: Add better observability with metrics and traces

Adds prometheus collectors that can be registered dynamically and also
refactors the http and grpc clients and servers to propage trace info.

https://github.com/cs3org/reva/pull/4166
8 changes: 4 additions & 4 deletions cmd/reva/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import (
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
"github.com/cs3org/reva/internal/http/services/datagateway"
ctxpkg "github.com/cs3org/reva/pkg/ctx"

"github.com/cs3org/reva/pkg/appctx"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/rhttp"
"github.com/cs3org/reva/pkg/utils"
"github.com/pkg/errors"
"github.com/studio-b12/gowebdav"
Expand Down Expand Up @@ -95,7 +95,7 @@ func downloadCommand() *command {

dataServerURL := p.DownloadEndpoint
// TODO(labkode): do a protocol switch
httpReq, err := rhttp.NewRequest(ctx, http.MethodGet, dataServerURL, nil)
httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, dataServerURL, nil)
if err != nil {
return err
}
Expand Down Expand Up @@ -183,7 +183,7 @@ func checkDownloadWebdavRef(protocols []*gateway.FileDownloadProtocol) (io.Reade
}

c := gowebdav.NewClient(p.DownloadEndpoint, "", "")
c.SetHeader(ctxpkg.TokenHeader, token)
c.SetHeader(appctx.TokenHeader, token)

reader, err := c.ReadStream(filePath)
if err != nil {
Expand Down
7 changes: 4 additions & 3 deletions cmd/reva/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ import (

gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
ctxpkg "github.com/cs3org/reva/pkg/ctx"
"github.com/cs3org/reva/pkg/appctx"

"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
ins "google.golang.org/grpc/credentials/insecure"
Expand All @@ -41,8 +42,8 @@ func getAuthContext() context.Context {
log.Println(err)
return ctx
}
ctx = ctxpkg.ContextSetToken(ctx, t)
ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.TokenHeader, t)
ctx = appctx.ContextSetToken(ctx, t)
ctx = metadata.AppendToOutgoingContext(ctx, appctx.TokenHeader, t)
return ctx
}

Expand Down
13 changes: 8 additions & 5 deletions cmd/reva/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package main

import (
"crypto/tls"
"flag"
"fmt"
"net/http"
Expand All @@ -27,7 +28,7 @@ import (
"time"

"github.com/c-bata/go-prompt"
"github.com/cs3org/reva/pkg/rhttp"
"github.com/cs3org/reva/pkg/httpclient"
)

var (
Expand All @@ -40,7 +41,7 @@ var (

gitCommit, buildDate, version, goVersion string

client *http.Client
client *httpclient.Client

commands = []*command{
versionCommand(),
Expand Down Expand Up @@ -124,9 +125,11 @@ func main() {
}
}

client = rhttp.GetHTTPClient(
rhttp.Insecure(insecuredatagateway),
rhttp.Timeout(time.Duration(timeout*int64(time.Hour))),
tr := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: insecuredatagateway}}

client = httpclient.New(
httpclient.RoundTripper(tr),
httpclient.Timeout(time.Duration(timeout*int64(time.Hour))),
)

generateMainUsage()
Expand Down
10 changes: 5 additions & 5 deletions cmd/reva/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ import (
// TODO(labkode): this should not come from this package.
"github.com/cs3org/reva/internal/grpc/services/storageprovider"
"github.com/cs3org/reva/internal/http/services/datagateway"

"github.com/cs3org/reva/pkg/appctx"
"github.com/cs3org/reva/pkg/crypto"
ctxpkg "github.com/cs3org/reva/pkg/ctx"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/rhttp"
"github.com/cs3org/reva/pkg/utils"
"github.com/eventials/go-tus"
"github.com/eventials/go-tus/memorystore"
Expand Down Expand Up @@ -147,7 +147,7 @@ func uploadCommand() *command {
dataServerURL := p.UploadEndpoint

if *protocolFlag == "simple" {
httpReq, err := rhttp.NewRequest(ctx, http.MethodPut, dataServerURL, fd)
httpReq, err := http.NewRequestWithContext(ctx, http.MethodPut, dataServerURL, fd)
if err != nil {
return err
}
Expand All @@ -170,7 +170,7 @@ func uploadCommand() *command {
// create the tus client.
c := tus.DefaultConfig()
c.Resume = true
c.HttpClient = client
c.HttpClient = client.GetNativeHTTP()
c.Store, err = memorystore.NewMemoryStore()
if err != nil {
return err
Expand Down Expand Up @@ -268,7 +268,7 @@ func checkUploadWebdavRef(protocols []*gateway.FileUploadProtocol, md os.FileInf
}

c := gowebdav.NewClient(p.UploadEndpoint, "", "")
c.SetHeader(ctxpkg.TokenHeader, token)
c.SetHeader(appctx.TokenHeader, token)
c.SetHeader("Upload-Length", strconv.FormatInt(md.Size(), 10))

if err = c.WriteStream(filePath, fd, 0700); err != nil {
Expand Down
27 changes: 16 additions & 11 deletions cmd/revad/runtime/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ import (
"github.com/cs3org/reva/internal/grpc/interceptors/appctx"
"github.com/cs3org/reva/internal/grpc/interceptors/auth"
"github.com/cs3org/reva/internal/grpc/interceptors/log"
"github.com/cs3org/reva/internal/grpc/interceptors/metrics"
"github.com/cs3org/reva/internal/grpc/interceptors/recovery"
"github.com/cs3org/reva/internal/grpc/interceptors/token"
"github.com/cs3org/reva/internal/grpc/interceptors/trace"
"github.com/cs3org/reva/internal/grpc/interceptors/useragent"
"github.com/cs3org/reva/pkg/rgrpc"
rtrace "github.com/cs3org/reva/pkg/trace"
"github.com/pkg/errors"
"github.com/rs/zerolog"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"google.golang.org/grpc"
)

Expand Down Expand Up @@ -75,24 +75,21 @@ func initGRPCInterceptors(conf map[string]map[string]any, unprotected []string,
return nil, nil, errors.Wrap(err, "error creating unary auth interceptor")
}

unaryInterceptors := []grpc.UnaryServerInterceptor{authUnary}
unaryInterceptors := []grpc.UnaryServerInterceptor{}
for _, t := range unaryTriples {
unaryInterceptors = append(unaryInterceptors, t.Interceptor)
logger.Info().Msgf("rgrpc: chaining grpc unary interceptor %s with priority %d", t.Name, t.Priority)
}

unaryInterceptors = append(unaryInterceptors,
otelgrpc.UnaryServerInterceptor(
otelgrpc.WithTracerProvider(rtrace.Provider),
otelgrpc.WithPropagators(rtrace.Propagator)),
)

unaryInterceptors = append([]grpc.UnaryServerInterceptor{
trace.NewUnary(),
metrics.NewUnary(),
appctx.NewUnary(*logger),
token.NewUnary(),
useragent.NewUnary(),
log.NewUnary(),
recovery.NewUnary(),
authUnary,
}, unaryInterceptors...)

streamTriples := []*streamInterceptorTriple{}
Expand Down Expand Up @@ -131,7 +128,8 @@ func initGRPCInterceptors(conf map[string]map[string]any, unprotected []string,
}

streamInterceptors = append([]grpc.StreamServerInterceptor{
authStream,
trace.NewStream(),
metrics.NewStream(),
appctx.NewStream(*logger),
token.NewStream(),
useragent.NewStream(),
Expand All @@ -142,7 +140,14 @@ func initGRPCInterceptors(conf map[string]map[string]any, unprotected []string,
return unaryInterceptors, streamInterceptors, nil
}

func grpcUnprotected(s map[string]rgrpc.Service) (unprotected []string) {
func grpcUnprotected(reflection bool, s map[string]rgrpc.Service) (unprotected []string) {
if reflection {
// TODO(labkode): do not hardcode service endpoint and try to obtain from reflection library
unprotected = append(unprotected,
"/grpc.reflection.v1alpha.ServerReflection",
"/grpc.reflection.v1.ServerReflection",
)
}
for _, svc := range s {
unprotected = append(unprotected, svc.UnprotectedEndpoints()...)
}
Expand Down
4 changes: 4 additions & 0 deletions cmd/revad/runtime/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"github.com/cs3org/reva/internal/http/interceptors/appctx"
"github.com/cs3org/reva/internal/http/interceptors/auth"
"github.com/cs3org/reva/internal/http/interceptors/log"
"github.com/cs3org/reva/internal/http/interceptors/metrics"
"github.com/cs3org/reva/internal/http/interceptors/trace"
"github.com/cs3org/reva/pkg/rhttp/global"
"github.com/pkg/errors"
"github.com/rs/zerolog"
Expand Down Expand Up @@ -70,6 +72,8 @@ func initHTTPMiddlewares(conf map[string]map[string]any, unprotected []string, l
authMiddle,
log.New(),
appctx.New(*logger),
metrics.New(),
trace.New(),
}

for _, triple := range triples {
Expand Down
1 change: 1 addition & 0 deletions cmd/revad/runtime/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
_ "github.com/cs3org/reva/pkg/ocm/share/repository/loader"
_ "github.com/cs3org/reva/pkg/permission/manager/loader"
_ "github.com/cs3org/reva/pkg/preferences/loader"
_ "github.com/cs3org/reva/pkg/prom/loader"
_ "github.com/cs3org/reva/pkg/publicshare/manager/loader"
_ "github.com/cs3org/reva/pkg/rhttp/datatx/manager/loader"
_ "github.com/cs3org/reva/pkg/share/cache/loader"
Expand Down
10 changes: 1 addition & 9 deletions cmd/revad/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import (
"github.com/cs3org/reva/pkg/rhttp/global"
"github.com/cs3org/reva/pkg/rserverless"
"github.com/cs3org/reva/pkg/sharedconf"
rtrace "github.com/cs3org/reva/pkg/trace"
"github.com/cs3org/reva/pkg/utils/list"
"github.com/cs3org/reva/pkg/utils/maps"
netutil "github.com/cs3org/reva/pkg/utils/net"
Expand Down Expand Up @@ -86,7 +85,6 @@ func New(config *config.Config, opt ...Option) (*Reva, error) {
if err := initCPUCount(config.Core, log); err != nil {
return nil, err
}
initTracing(config.Core)

if opts.PidFile == "" {
return nil, errors.New("pid file not provided")
Expand Down Expand Up @@ -347,12 +345,6 @@ func handlePIDFlag(l *zerolog.Logger, pidFile string) (*grace.Watcher, error) {
return w, nil
}

func initTracing(conf *config.Core) {
if conf.TracingEnabled {
rtrace.SetTraceProvider(conf.TracingCollector, conf.TracingEndpoint, conf.TracingServiceName)
}
}

// adjustCPU parses string cpu and sets GOMAXPROCS
// according to its value. It accepts either
// a number (e.g. 3) or a percent (e.g. 50%).
Expand Down Expand Up @@ -411,7 +403,7 @@ func newServers(ctx context.Context, grpc []*config.GRPC, http []*config.HTTP, l
if err != nil {
return nil, err
}
unaryChain, streamChain, err := initGRPCInterceptors(cfg.Interceptors, grpcUnprotected(services), log)
unaryChain, streamChain, err := initGRPCInterceptors(cfg.Interceptors, grpcUnprotected(cfg.EnableReflection, services), log)
if err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ description: >
# _struct: config_

{{% dir name="provider_domain" type="string" default="The same domain registered in the provider authorizer" %}}
[[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/ocminvitemanager/ocminvitemanager.go#L62)
[[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/ocminvitemanager/ocminvitemanager.go#L61)
{{< highlight toml >}}
[grpc.services.ocminvitemanager]
provider_domain = "The same domain registered in the provider authorizer"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ description: >
# _struct: config_

{{% dir name="provider_domain" type="string" default="The same domain registered in the provider authorizer" %}}
[[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/ocmshareprovider/ocmshareprovider.go#L70)
[[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/ocmshareprovider/ocmshareprovider.go#L71)
{{< highlight toml >}}
[grpc.services.ocmshareprovider]
provider_domain = "The same domain registered in the provider authorizer"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,31 @@ description: >
# _struct: config_

{{% dir name="mount_path" type="string" default="/" %}}
The path where the file system would be mounted. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L63)
The path where the file system would be mounted. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L61)
{{< highlight toml >}}
[grpc.services.storageprovider]
mount_path = "/"
{{< /highlight >}}
{{% /dir %}}

{{% dir name="mount_id" type="string" default="-" %}}
The ID of the mounted file system. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L64)
The ID of the mounted file system. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L62)
{{< highlight toml >}}
[grpc.services.storageprovider]
mount_id = "-"
{{< /highlight >}}
{{% /dir %}}

{{% dir name="driver" type="string" default="localhome" %}}
The storage driver to be used. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L65)
The storage driver to be used. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L63)
{{< highlight toml >}}
[grpc.services.storageprovider]
driver = "localhome"
{{< /highlight >}}
{{% /dir %}}

{{% dir name="drivers" type="map[string]map[string]interface{}" default="localhome" %}}
[[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L66)
[[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L64)
{{< highlight toml >}}
[grpc.services.storageprovider.drivers.localhome]
root = "/var/tmp/reva/"
Expand All @@ -44,39 +44,39 @@ user_layout = "{{.Username}}"
{{% /dir %}}

{{% dir name="tmp_folder" type="string" default="/var/tmp" %}}
Path to temporary folder. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L67)
Path to temporary folder. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L65)
{{< highlight toml >}}
[grpc.services.storageprovider]
tmp_folder = "/var/tmp"
{{< /highlight >}}
{{% /dir %}}

{{% dir name="data_server_url" type="string" default="http://localhost/data" %}}
The URL for the data server. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L68)
The URL for the data server. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L66)
{{< highlight toml >}}
[grpc.services.storageprovider]
data_server_url = "http://localhost/data"
{{< /highlight >}}
{{% /dir %}}

{{% dir name="expose_data_server" type="bool" default=false %}}
Whether to expose data server. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L69)
Whether to expose data server. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L67)
{{< highlight toml >}}
[grpc.services.storageprovider]
expose_data_server = false
{{< /highlight >}}
{{% /dir %}}

{{% dir name="available_checksums" type="map[string]uint32" default=nil %}}
List of available checksums. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L70)
List of available checksums. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L68)
{{< highlight toml >}}
[grpc.services.storageprovider]
available_checksums = nil
{{< /highlight >}}
{{% /dir %}}

{{% dir name="custom_mime_types_json" type="string" default="nil" %}}
An optional mapping file with the list of supported custom file extensions and corresponding mime types. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L71)
An optional mapping file with the list of supported custom file extensions and corresponding mime types. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/storageprovider/storageprovider.go#L69)
{{< highlight toml >}}
[grpc.services.storageprovider]
custom_mime_types_json = "nil"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ description: >
# _struct: Config_

{{% dir name="insecure" type="bool" default=false %}}
Whether to skip certificate checks when sending requests. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/appprovider/appprovider.go#L56)
Whether to skip certificate checks when sending requests. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/appprovider/appprovider.go#L57)
{{< highlight toml >}}
[http.services.appprovider]
insecure = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ description: >
# _struct: Config_

{{% dir name="insecure" type="bool" default=false %}}
Whether to skip certificate checks when sending requests. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/archiver/handler.go#L63)
Whether to skip certificate checks when sending requests. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/archiver/handler.go#L64)
{{< highlight toml >}}
[http.services.archiver]
insecure = false
Expand Down
Loading

0 comments on commit 084c1b5

Please sign in to comment.