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

New observability + general refactor of http/grpc clients #4166

Merged
merged 30 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
d0b9ba7
remove unused fs and tracing
labkode Sep 6, 2023
b132e3a
add helloworld example and fix auth logic to bail out early
labkode Sep 6, 2023
7a38177
remove hack on appctx and add traceid
labkode Sep 6, 2023
e94f1e9
Merge remote-tracking branch 'up/master' into new-observability
labkode Sep 11, 2023
8a2831e
configurable prom collectors
labkode Sep 12, 2023
bc289b0
grpc reflection working
labkode Sep 13, 2023
fcf9030
wire http traffic
labkode Sep 13, 2023
16b297e
intrument outgoing grpc calls
labkode Sep 13, 2023
28c69bb
add stuff
labkode Sep 13, 2023
5d32635
merge appctx and ctx
labkode Sep 26, 2023
21e2423
refactor rhttp to httpclient
labkode Sep 27, 2023
a46da11
tidy go.mod
labkode Sep 27, 2023
b6807a0
remove drone tests
labkode Sep 27, 2023
e68c0ff
Merge remote-tracking branch 'up/master' into attempt
labkode Sep 27, 2023
94e7748
add changelog
labkode Sep 27, 2023
210803d
fix test
labkode Sep 27, 2023
5dec790
compiles after merge upstream
labkode Oct 9, 2023
6fb11d1
go 1.21
labkode Oct 9, 2023
c0c86fc
pass tests
labkode Oct 9, 2023
861f028
fix sql tests
labkode Oct 9, 2023
e9b70c5
go fmt
labkode Oct 9, 2023
a5c056e
more linters
labkode Oct 9, 2023
197d546
make all linters happy
labkode Oct 9, 2023
b503ae9
update docs
labkode Oct 9, 2023
6318ff2
add back ceph example
labkode Oct 10, 2023
0d32711
remove otelhttp library
labkode Oct 10, 2023
df01798
remove files and add tests
labkode Oct 10, 2023
f690442
Merge remote-tracking branch 'up/master' into new-observability
labkode Oct 10, 2023
e160591
align with upstream
labkode Oct 10, 2023
9608291
Merge remote-tracking branch 'up/master' into new-observability
labkode Oct 10, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading