Skip to content

Commit

Permalink
feat: code instrumentation (#207)
Browse files Browse the repository at this point in the history
* feat: metrics for missing metadata keys

* fix: typo

* feat: add counter metric for failed resource creation

* feat: add counter metric for failed relation creation

* feat: add /metrics endpoint to port a separate port
  • Loading branch information
ishanarya0 authored Feb 28, 2023
1 parent 3ad33d2 commit 953d2b7
Show file tree
Hide file tree
Showing 11 changed files with 211 additions and 4 deletions.
1 change: 1 addition & 0 deletions config/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ app:
port: 8000
grpc:
port: 8001
metrics_port: 9000
identity_proxy_header: X-Shield-Email
# full path prefixed with scheme where resources config yaml files are kept
# e.g.:
Expand Down
1 change: 1 addition & 0 deletions core/user/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var (
ErrConflict = errors.New("user already exist")
ErrEmptyKey = errors.New("empty key")
ErrKeyAlreadyExists = errors.New("key already exist")
ErrKeyDoesNotExists = errors.New("key does not exist")
ErrMissingEmail = errors.New("user email is missing")
ErrInvalidUUID = errors.New("invalid syntax of uuid")
)
18 changes: 17 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ module github.com/odpf/shield
go 1.18

require (
contrib.go.opencensus.io/exporter/ocagent v0.7.0
contrib.go.opencensus.io/exporter/prometheus v0.4.2
github.com/MakeNowJust/heredoc v1.0.0
github.com/abbot/go-http-auth v0.4.0
github.com/authzed/authzed-go v0.7.1-0.20221109204547-1aa903788b3b
Expand All @@ -27,6 +29,7 @@ require (
github.com/mcuadros/go-defaults v1.2.0
github.com/mitchellh/mapstructure v1.5.0
github.com/newrelic/go-agent v3.20.2+incompatible
github.com/newrelic/newrelic-opencensus-exporter-go v0.4.0
github.com/odpf/salt v0.2.5-0.20221130085531-51c81815f7d6
github.com/ory/dockertest v3.3.5+incompatible
github.com/pkg/errors v0.9.1
Expand All @@ -37,6 +40,7 @@ require (
github.com/spf13/cobra v1.6.1
github.com/stretchr/testify v1.8.1
github.com/tidwall/gjson v1.14.4
go.opencensus.io v0.24.0
go.uber.org/zap v1.24.0
gocloud.dev v0.28.0
golang.org/x/exp v0.0.0-20230108222341-4b8118a2686a
Expand Down Expand Up @@ -64,9 +68,12 @@ require (
github.com/antlr/antlr4/runtime/Go/antlr v1.4.10 // indirect
github.com/aymanbagabas/go-osc52 v1.2.1 // indirect
github.com/aymerick/douceur v0.2.0 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/briandowns/spinner v1.20.0 // indirect
github.com/cenkalti/backoff v2.2.1+incompatible // indirect
github.com/census-instrumentation/opencensus-proto v0.4.1 // indirect
github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d // indirect
github.com/cespare/xxhash/v2 v2.1.2 // indirect
github.com/charmbracelet/glamour v0.6.0 // indirect
github.com/cli/safeexec v1.0.1 // indirect
github.com/containerd/continuity v0.3.0 // indirect
Expand All @@ -79,6 +86,8 @@ require (
github.com/fatih/color v1.13.0 // indirect
github.com/felixge/fgprof v0.9.3 // indirect
github.com/fsnotify/fsnotify v1.6.0 // indirect
github.com/go-kit/log v0.2.1 // indirect
github.com/go-logfmt/logfmt v0.5.1 // indirect
github.com/go-sql-driver/mysql v1.7.0 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/google/cel-go v0.13.0 // indirect
Expand Down Expand Up @@ -106,17 +115,24 @@ require (
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-isatty v0.0.17 // indirect
github.com/mattn/go-runewidth v0.0.14 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/microcosm-cc/bluemonday v1.0.21 // indirect
github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db // indirect
github.com/muesli/reflow v0.3.0 // indirect
github.com/muesli/termenv v0.13.0 // indirect
github.com/newrelic/newrelic-telemetry-sdk-go v0.2.0 // indirect
github.com/olekukonko/tablewriter v0.0.5 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/opencontainers/image-spec v1.0.2 // indirect
github.com/opencontainers/runc v1.1.2 // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pelletier/go-toml/v2 v2.0.6 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_golang v1.13.1 // indirect
github.com/prometheus/client_model v0.3.0 // indirect
github.com/prometheus/common v0.37.0 // indirect
github.com/prometheus/procfs v0.8.0 // indirect
github.com/prometheus/statsd_exporter v0.22.7 // indirect
github.com/rivo/uniseg v0.4.3 // indirect
github.com/rs/zerolog v1.28.0 // indirect
github.com/russross/blackfriday/v2 v2.1.0 // indirect
Expand All @@ -133,10 +149,10 @@ require (
github.com/tidwall/pretty v1.2.1 // indirect
github.com/yuin/goldmark v1.5.3 // indirect
github.com/yuin/goldmark-emoji v1.0.1 // indirect
go.opencensus.io v0.24.0 // indirect
go.uber.org/atomic v1.10.0 // indirect
go.uber.org/multierr v1.9.0 // indirect
golang.org/x/crypto v0.5.0 // indirect
golang.org/x/sync v0.1.0 // indirect
golang.org/x/sys v0.4.0 // indirect
golang.org/x/term v0.4.0 // indirect
golang.org/x/text v0.6.0 // indirect
Expand Down
30 changes: 30 additions & 0 deletions go.sum

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions internal/api/v1beta1/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@ import (
"strings"

grpczap "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
"go.opencensus.io/stats"
"go.opencensus.io/tag"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/odpf/shield/core/user"
"github.com/odpf/shield/pkg/metadata"
"github.com/odpf/shield/pkg/telemetry"
"github.com/odpf/shield/pkg/uuid"
shieldv1beta1 "github.com/odpf/shield/proto/v1beta1"
)
Expand Down Expand Up @@ -65,6 +68,7 @@ func (h Handler) ListUsers(ctx context.Context, request *shieldv1beta1.ListUsers

func (h Handler) CreateUser(ctx context.Context, request *shieldv1beta1.CreateUserRequest) (*shieldv1beta1.CreateUserResponse, error) {
logger := grpczap.Extract(ctx)
ctx, err := tag.New(ctx, tag.Insert(telemetry.KeyMethod, "CreateUser"))

currentUserEmail, ok := user.GetEmailFromContext(ctx)
if !ok {
Expand Down Expand Up @@ -103,6 +107,14 @@ func (h Handler) CreateUser(ctx context.Context, request *shieldv1beta1.CreateUs
switch {
case errors.Is(err, user.ErrConflict):
return nil, grpcConflictError
case errors.Is(errors.Unwrap(err), user.ErrKeyDoesNotExists):
missingKey := strings.Split(err.Error(), ":")
if len(missingKey) == 2 {
ctx, _ = tag.New(ctx, tag.Upsert(telemetry.KeyMissingKey, missingKey[1]))
}
stats.Record(ctx, telemetry.MMissingMetadataKeys.M(1))

return nil, grpcBadBodyError
default:
return nil, grpcInternalServerError
}
Expand Down
36 changes: 35 additions & 1 deletion internal/proxy/hook/authz/authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"strings"

"github.com/mitchellh/mapstructure"
"go.opencensus.io/stats"
"go.opencensus.io/tag"

"github.com/odpf/salt/log"
"github.com/odpf/shield/core/namespace"
Expand All @@ -17,6 +19,7 @@ import (
"github.com/odpf/shield/internal/proxy/hook"
"github.com/odpf/shield/internal/proxy/middleware"
"github.com/odpf/shield/pkg/body_extractor"
"github.com/odpf/shield/pkg/telemetry"
)

type ResourceService interface {
Expand Down Expand Up @@ -82,6 +85,20 @@ func (a Authz) ServeHook(res *http.Response, err error) (*http.Response, error)
return a.escape.ServeHook(res, err)
}

isResourceCreated := false
attributes := map[string]interface{}{}

defer func(isResourceCreated bool, ctx context.Context, attributes map[string]interface{}) {
if !isResourceCreated {
requestDetail := fmt.Sprintf("[status: %d, request: %s, attributes: %s ]", res.StatusCode, res.Request.Method+"@"+res.Request.URL.Host, attributes)
ctx, err := tag.New(ctx, tag.Insert(telemetry.KeyMethod, "ServeHook"), tag.Insert(telemetry.KeyRequestDetails, requestDetail))
if err != nil {
a.log.Debug("failed to add metrics tags: ", err.Error())
}
stats.Record(ctx, telemetry.MResourceFailedToCreate.M(1))
}
}(isResourceCreated, res.Request.Context(), attributes)

ruleFromRequest, ok := hook.ExtractRule(res.Request)
if !ok {
return a.next.ServeHook(res, nil)
Expand All @@ -101,7 +118,6 @@ func (a Authz) ServeHook(res *http.Response, err error) (*http.Response, error)
return a.next.ServeHook(res, fmt.Errorf("namespace variable not defined in rules"))
}

attributes := map[string]interface{}{}
attributes["namespace"] = ruleFromRequest.Backend.Namespace

identityProxyHeaderValue := res.Request.Header.Get(a.identityProxyHeaderKey)
Expand Down Expand Up @@ -208,12 +224,22 @@ func (a Authz) ServeHook(res *http.Response, err error) (*http.Response, error)
a.log.Error(err.Error())
return a.escape.ServeHook(res, fmt.Errorf(err.Error()))
}

isResourceCreated = true
a.log.Info(fmt.Sprintf("Resource %s created with ID %s", newResource.URN, newResource.Idxa))

for _, rel := range config.Relations {
subjectId, err := getAttributesValues(attributes[rel.SubjectIDAttribute])
if err != nil {
a.log.Error(fmt.Sprintf("cannot create relation: %s not found in attributes", rel.SubjectIDAttribute))

relationDetail := fmt.Sprintf("%s to %s %s on %s", rel.Role, rel.SubjectPrincipal, rel.SubjectIDAttribute, newResource.Name)
ctx, err := tag.New(res.Request.Context(), tag.Insert(telemetry.KeyMethod, "ServeHook"), tag.Insert(telemetry.KeyRelationDetails, relationDetail))
if err != nil {
a.log.Debug("failed to add metrics tags: ", err.Error())
}
stats.Record(ctx, telemetry.MRelationFailedToCreate.M(1))

continue
}

Expand All @@ -230,6 +256,14 @@ func (a Authz) ServeHook(res *http.Response, err error) (*http.Response, error)
})
if err != nil {
a.log.Error(err.Error())

relationDetail := fmt.Sprintf("%s to %s %s on %s", rel.Role, rel.SubjectPrincipal, rel.SubjectIDAttribute, newResource.Name)
ctx, err := tag.New(res.Request.Context(), tag.Insert(telemetry.KeyMethod, "ServeHook"), tag.Insert(telemetry.KeyRelationDetails, relationDetail))
if err != nil {
a.log.Debug("failed to add metrics tags: ", err.Error())
}
stats.Record(ctx, telemetry.MRelationFailedToCreate.M(1))

return a.escape.ServeHook(res, fmt.Errorf(err.Error()))
}

Expand Down
11 changes: 10 additions & 1 deletion internal/server/config.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package server

import "fmt"
import (
"fmt"

"github.com/odpf/shield/pkg/telemetry"
)

type GRPCConfig struct {
Port int `mapstructure:"port" default:"8081"`
Expand All @@ -17,6 +21,9 @@ type Config struct {
// GRPC Config
GRPC GRPCConfig `mapstructure:"grpc"`

//metrics port
MetricsPort int `yaml:"metrics_port" mapstructure:"metrics_port" default:"9000"`

// the network interface to listen on
Host string `yaml:"host" mapstructure:"host" default:"127.0.0.1"`

Expand All @@ -43,4 +50,6 @@ type Config struct {
// ResourcesPathSecretSecret could be a env name, file path or actual value required
// to access ResourcesPathSecretPath files
ResourcesConfigPathSecret string `yaml:"resources_config_path_secret" mapstructure:"resources_config_path_secret"`

TelemetryConfig telemetry.Config `yaml:"telemetry_config" mapstructure:"telemetry_config"`
}
17 changes: 16 additions & 1 deletion internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/odpf/shield/internal/api/v1beta1"
"github.com/odpf/shield/internal/server/grpc_interceptors"
"github.com/odpf/shield/internal/server/health"
"github.com/odpf/shield/pkg/telemetry"
shieldv1beta1 "github.com/odpf/shield/proto/v1beta1"
"go.uber.org/zap"
"google.golang.org/grpc"
Expand Down Expand Up @@ -76,7 +77,15 @@ func Serve(
return err
}

logger.Info("[shield] api server starting", "http-port", cfg.Port, "grpc-port", cfg.GRPC.Port)
pe, err := telemetry.SetupOpenCensus(ctx, cfg.TelemetryConfig)
if err != nil {
logger.Error("failed to setup OpenCensus", "err", err)
}

httpMuxMetrics := http.NewServeMux()
httpMuxMetrics.Handle("/metrics", pe)

logger.Info("[shield] api server starting", "http-port", cfg.Port, "grpc-port", cfg.GRPC.Port, "metrics-port", cfg.MetricsPort)

if err := mux.Serve(
ctx,
Expand All @@ -86,6 +95,12 @@ func Serve(
WriteTimeout: 120 * time.Second,
MaxHeaderBytes: 1 << 20,
}),
mux.WithHTTPTarget(fmt.Sprintf(":%d", cfg.MetricsPort), &http.Server{
Handler: httpMuxMetrics,
ReadTimeout: 120 * time.Second,
WriteTimeout: 120 * time.Second,
MaxHeaderBytes: 1 << 20,
}),
mux.WithGRPCTarget(fmt.Sprintf(":%d", cfg.GRPC.Port), grpcServer),
mux.WithGracePeriod(5*time.Second),
); !errors.Is(err, context.Canceled) {
Expand Down
9 changes: 9 additions & 0 deletions internal/store/postgres/user_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"errors"
"fmt"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -228,6 +229,14 @@ func (r UserRepository) Create(ctx context.Context, usr user.User) (user.User, e
switch {
case errors.Is(err, errDuplicateKey):
return user.User{}, user.ErrConflict
case errors.Is(err, errForeignKeyViolation):
re := regexp.MustCompile(`\(([^)]+)\) `)
match := re.FindStringSubmatch(err.Error())
if len(match) > 1 {
return user.User{}, fmt.Errorf("%w:%s", user.ErrKeyDoesNotExists, match[1])
}
return user.User{}, user.ErrKeyDoesNotExists

default:
tx.Rollback()
return user.User{}, err
Expand Down
74 changes: 74 additions & 0 deletions pkg/telemetry/opencensus.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package telemetry

import (
"context"

"contrib.go.opencensus.io/exporter/prometheus"
"go.opencensus.io/stats"
"go.opencensus.io/stats/view"
"go.opencensus.io/tag"
)

var (
// The number of new metadata keys
MMissingMetadataKeys = stats.Int64("metadata-keys/counter", "The number of missing metadata keys", "1")
MResourceFailedToCreate = stats.Int64("resource-failed-to-create/counter", "The number of resources failed to be created", "1")
MRelationFailedToCreate = stats.Int64("relation-failed-to-create/counter", "The number of relations failed to be created", "1")
)

var (
KeyMethod, _ = tag.NewKey("method")
KeyMissingKey, _ = tag.NewKey("missing-key")
KeyRequestDetails, _ = tag.NewKey("request-details")
KeyRelationDetails, _ = tag.NewKey("relation-details")
)

var (
MissingMetadataKeysView = &view.View{
Name: "metadata-keys/counter",
Measure: MMissingMetadataKeys,
Description: "The number of missing metadata keys",

Aggregation: view.Count(),
TagKeys: []tag.Key{KeyMethod, KeyMissingKey}}

ResourceFailedToCreateView = &view.View{
Name: "resource-failed-to-create/counter",
Measure: MResourceFailedToCreate,
Description: "The number of resources failed to be created",

Aggregation: view.Count(),
TagKeys: []tag.Key{KeyMethod, KeyRequestDetails}}

RelationFailedToCreateView = &view.View{
Name: "relation-failed-to-create/counter",
Measure: MRelationFailedToCreate,
Description: "The number of relations failed to be created",

Aggregation: view.Count(),
TagKeys: []tag.Key{KeyMethod, KeyRelationDetails}}
)

func SetupOpenCensus(ctx context.Context, cfg Config) (*prometheus.Exporter, error) {
if err := setupViews(); err != nil {
return nil, err
}

pe, err := prometheus.NewExporter(prometheus.Options{
Namespace: cfg.ServiceName,
})
if err != nil {
return nil, err
}
view.RegisterExporter(pe)
return pe, nil
}

func setupViews() error {
err := view.Register(MissingMetadataKeysView, ResourceFailedToCreateView, RelationFailedToCreateView)
if err != nil {
return err
}

return nil
}
6 changes: 6 additions & 0 deletions pkg/telemetry/telemetry.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package telemetry

type Config struct {
// OpenCensus exporter configurations.
ServiceName string `yaml:"service_name" mapstructure:"service_name"`
}

0 comments on commit 953d2b7

Please sign in to comment.