Skip to content

Commit b340634

Browse files
coadlercoadler
authored
feat: add rbac tracing (coder#4093)
1 parent 1bca269 commit b340634

14 files changed

+79
-34
lines changed

‎coderd/audit/request.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import (
1313
"cdr.dev/slog"
1414
"github.com/coder/coder/coderd/database"
1515
"github.com/coder/coder/coderd/features"
16-
"github.com/coder/coder/coderd/httpapi"
1716
"github.com/coder/coder/coderd/httpmw"
17+
"github.com/coder/coder/coderd/tracing"
1818
)
1919

2020
type RequestParams struct {
@@ -93,9 +93,9 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
9393
// that should be deferred, causing the audit log to be committed when the
9494
// handler returns.
9595
func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request[T], func()) {
96-
sw, ok := w.(*httpapi.StatusWriter)
96+
sw, ok := w.(*tracing.StatusWriter)
9797
if !ok {
98-
panic("dev error: http.ResponseWriter is not *httpapi.StatusWriter")
98+
panic("dev error: http.ResponseWriter is not *tracing.StatusWriter")
9999
}
100100

101101
req := &Request[T]{

‎coderd/coderd.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ func New(options *Options) *API {
199199
apps := func(r chi.Router) {
200200
r.Use(
201201
httpmw.RateLimitPerMinute(options.APIRateLimit),
202-
tracing.HTTPMW(api.TracerProvider, "coderd.http"),
202+
tracing.HTTPMW(api.TracerProvider),
203203
httpmw.ExtractAPIKey(options.Database, oauthConfigs, true),
204204
httpmw.ExtractUserParam(api.Database),
205205
// Extracts the <workspace.agent> from the url
@@ -229,7 +229,7 @@ func New(options *Options) *API {
229229
r.Use(
230230
// Specific routes can specify smaller limits.
231231
httpmw.RateLimitPerMinute(options.APIRateLimit),
232-
tracing.HTTPMW(api.TracerProvider, "coderd.http"),
232+
tracing.HTTPMW(api.TracerProvider),
233233
)
234234
r.Get("/", func(w http.ResponseWriter, r *http.Request) {
235235
httpapi.Write(w, http.StatusOK, codersdk.Response{

‎coderd/httpmw/logger.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@ import (
66

77
"cdr.dev/slog"
88
"github.com/coder/coder/coderd/httpapi"
9+
"github.com/coder/coder/coderd/tracing"
910
)
1011

1112
func Logger(log slog.Logger) func(next http.Handler) http.Handler {
1213
return func(next http.Handler) http.Handler {
1314
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1415
start := time.Now()
15-
sw := &httpapi.StatusWriter{ResponseWriter: w}
16+
sw := &tracing.StatusWriter{ResponseWriter: w}
1617

1718
httplog := log.With(
1819
slog.F("host", httpapi.RequestHost(r)),

‎coderd/httpmw/prometheus.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/go-chi/chi/v5"
99

1010
"github.com/coder/coder/coderd/httpapi"
11+
"github.com/coder/coder/coderd/tracing"
1112

1213
"github.com/prometheus/client_golang/prometheus"
1314
"github.com/prometheus/client_golang/prometheus/promauto"
@@ -67,9 +68,9 @@ func Prometheus(register prometheus.Registerer) func(http.Handler) http.Handler
6768
rctx = chi.RouteContext(r.Context())
6869
)
6970

70-
sw, ok := w.(*httpapi.StatusWriter)
71+
sw, ok := w.(*tracing.StatusWriter)
7172
if !ok {
72-
panic("dev error: http.ResponseWriter is not *httpapi.StatusWriter")
73+
panic("dev error: http.ResponseWriter is not *tracing.StatusWriter")
7374
}
7475

7576
var (

‎coderd/httpmw/prometheus_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import (
1010
"github.com/prometheus/client_golang/prometheus"
1111
"github.com/stretchr/testify/require"
1212

13-
"github.com/coder/coder/coderd/httpapi"
1413
"github.com/coder/coder/coderd/httpmw"
14+
"github.com/coder/coder/coderd/tracing"
1515
)
1616

1717
func TestPrometheus(t *testing.T) {
@@ -20,7 +20,7 @@ func TestPrometheus(t *testing.T) {
2020
t.Parallel()
2121
req := httptest.NewRequest("GET", "/", nil)
2222
req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, chi.NewRouteContext()))
23-
res := &httpapi.StatusWriter{ResponseWriter: httptest.NewRecorder()}
23+
res := &tracing.StatusWriter{ResponseWriter: httptest.NewRecorder()}
2424
reg := prometheus.NewRegistry()
2525
httpmw.Prometheus(reg)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
2626
w.WriteHeader(http.StatusOK)

‎coderd/httpmw/recover.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"cdr.dev/slog"
99
"github.com/coder/coder/coderd/httpapi"
10+
"github.com/coder/coder/coderd/tracing"
1011
)
1112

1213
func Recover(log slog.Logger) func(h http.Handler) http.Handler {
@@ -22,7 +23,7 @@ func Recover(log slog.Logger) func(h http.Handler) http.Handler {
2223
)
2324

2425
var hijacked bool
25-
if sw, ok := w.(*httpapi.StatusWriter); ok {
26+
if sw, ok := w.(*tracing.StatusWriter); ok {
2627
hijacked = sw.Hijacked
2728
}
2829

‎coderd/httpmw/recover_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
"github.com/stretchr/testify/require"
99

1010
"cdr.dev/slog/sloggers/slogtest"
11-
"github.com/coder/coder/coderd/httpapi"
1211
"github.com/coder/coder/coderd/httpmw"
12+
"github.com/coder/coder/coderd/tracing"
1313
)
1414

1515
func TestRecover(t *testing.T) {
@@ -60,7 +60,7 @@ func TestRecover(t *testing.T) {
6060
var (
6161
log = slogtest.Make(t, nil)
6262
r = httptest.NewRequest("GET", "/", nil)
63-
w = &httpapi.StatusWriter{
63+
w = &tracing.StatusWriter{
6464
ResponseWriter: httptest.NewRecorder(),
6565
Hijacked: c.Hijack,
6666
}

‎coderd/rbac/authz.go

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import (
44
"context"
55
_ "embed"
66

7+
"github.com/open-policy-agent/opa/rego"
8+
"go.opentelemetry.io/otel/attribute"
9+
"go.opentelemetry.io/otel/trace"
710
"golang.org/x/xerrors"
811

9-
"github.com/open-policy-agent/opa/rego"
12+
"github.com/coder/coder/coderd/tracing"
1013
)
1114

1215
type Authorizer interface {
@@ -22,6 +25,13 @@ type PreparedAuthorized interface {
2225
// the elements the subject does not have permission for. All objects must be
2326
// of the same type.
2427
func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, subjRoles []string, action Action, objects []O) ([]O, error) {
28+
ctx, span := tracing.StartSpan(ctx, trace.WithAttributes(
29+
attribute.String("subject_id", subjID),
30+
attribute.StringSlice("subject_roles", subjRoles),
31+
attribute.Int("num_objects", len(objects)),
32+
))
33+
defer span.End()
34+
2535
if len(objects) == 0 {
2636
// Nothing to filter
2737
return objects, nil
@@ -34,8 +44,7 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, sub
3444
return nil, xerrors.Errorf("prepare: %w", err)
3545
}
3646

37-
for i := range objects {
38-
object := objects[i]
47+
for _, object := range objects {
3948
rbacObj := object.RBACObject()
4049
if rbacObj.Type != objectType {
4150
return nil, xerrors.Errorf("object types must be uniform across the set (%s), found %s", objectType, object.RBACObject().Type)
@@ -45,6 +54,7 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subjID string, sub
4554
filtered = append(filtered, object)
4655
}
4756
}
57+
4858
return filtered, nil
4959
}
5060

@@ -93,6 +103,9 @@ func (a RegoAuthorizer) ByRoleName(ctx context.Context, subjectID string, roleNa
93103
// Authorize allows passing in custom Roles.
94104
// This is really helpful for unit testing, as we can create custom roles to exercise edge cases.
95105
func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles []Role, action Action, object Object) error {
106+
ctx, span := tracing.StartSpan(ctx)
107+
defer span.End()
108+
96109
input := map[string]interface{}{
97110
"subject": authSubject{
98111
ID: subjectID,
@@ -117,6 +130,9 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subjectID string, roles [
117130
// Prepare will partially execute the rego policy leaving the object fields unknown (except for the type).
118131
// This will vastly speed up performance if batch authorization on the same type of objects is needed.
119132
func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Role, action Action, objectType string) (*PartialAuthorizer, error) {
133+
ctx, span := tracing.StartSpan(ctx)
134+
defer span.End()
135+
120136
auth, err := newPartialAuthorizer(ctx, subjectID, roles, action, objectType)
121137
if err != nil {
122138
return nil, xerrors.Errorf("new partial authorizer: %w", err)
@@ -126,6 +142,9 @@ func (RegoAuthorizer) Prepare(ctx context.Context, subjectID string, roles []Rol
126142
}
127143

128144
func (a RegoAuthorizer) PrepareByRoleName(ctx context.Context, subjectID string, roleNames []string, action Action, objectType string) (PreparedAuthorized, error) {
145+
ctx, span := tracing.StartSpan(ctx)
146+
defer span.End()
147+
129148
roles, err := RolesByNames(roleNames)
130149
if err != nil {
131150
return nil, err

‎coderd/rbac/partial.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"golang.org/x/xerrors"
77

88
"github.com/open-policy-agent/opa/rego"
9+
10+
"github.com/coder/coder/coderd/tracing"
911
)
1012

1113
type PartialAuthorizer struct {
@@ -24,6 +26,9 @@ type PartialAuthorizer struct {
2426
}
2527

2628
func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, action Action, objectType string) (*PartialAuthorizer, error) {
29+
ctx, span := tracing.StartSpan(ctx)
30+
defer span.End()
31+
2732
input := map[string]interface{}{
2833
"subject": authSubject{
2934
ID: subjectID,
@@ -83,6 +88,9 @@ func newPartialAuthorizer(ctx context.Context, subjectID string, roles []Role, a
8388

8489
// Authorize authorizes a single object using the partially prepared queries.
8590
func (a PartialAuthorizer) Authorize(ctx context.Context, object Object) error {
91+
ctx, span := tracing.StartSpan(ctx)
92+
defer span.End()
93+
8694
if a.alwaysTrue {
8795
return nil
8896
}

‎coderd/tracing/httpmw.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
11
package tracing
22

33
import (
4+
"context"
45
"fmt"
56
"net/http"
67

78
"github.com/go-chi/chi/v5"
89
semconv "go.opentelemetry.io/otel/semconv/v1.10.0"
910
"go.opentelemetry.io/otel/trace"
10-
11-
"github.com/coder/coder/coderd/httpapi"
1211
)
1312

1413
// HTTPMW adds tracing to http routes.
15-
func HTTPMW(tracerProvider trace.TracerProvider, name string) func(http.Handler) http.Handler {
14+
func HTTPMW(tracerProvider trace.TracerProvider) func(http.Handler) http.Handler {
1615
return func(next http.Handler) http.Handler {
1716
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
1817
if tracerProvider == nil {
@@ -21,13 +20,13 @@ func HTTPMW(tracerProvider trace.TracerProvider, name string) func(http.Handler)
2120
}
2221

2322
// start span with default span name. Span name will be updated to "method route" format once request finishes.
24-
ctx, span := tracerProvider.Tracer(name).Start(r.Context(), fmt.Sprintf("%s %s", r.Method, r.RequestURI))
23+
ctx, span := tracerProvider.Tracer("").Start(r.Context(), fmt.Sprintf("%s %s", r.Method, r.RequestURI))
2524
defer span.End()
2625
r = r.WithContext(ctx)
2726

28-
sw, ok := rw.(*httpapi.StatusWriter)
27+
sw, ok := rw.(*StatusWriter)
2928
if !ok {
30-
panic(fmt.Sprintf("ResponseWriter not a *httpapi.StatusWriter; got %T", rw))
29+
panic(fmt.Sprintf("ResponseWriter not a *tracing.StatusWriter; got %T", rw))
3130
}
3231

3332
// pass the span through the request context and serve the request to the next middleware
@@ -53,9 +52,12 @@ func EndHTTPSpan(r *http.Request, status int, span trace.Span) {
5352
status = http.StatusOK
5453
}
5554
span.SetAttributes(semconv.HTTPStatusCodeKey.Int(status))
56-
spanStatus, spanMessage := semconv.SpanStatusFromHTTPStatusCodeAndSpanKind(status, trace.SpanKindServer)
57-
span.SetStatus(spanStatus, spanMessage)
55+
span.SetStatus(semconv.SpanStatusFromHTTPStatusCodeAndSpanKind(status, trace.SpanKindServer))
5856

5957
// finally end span
6058
span.End()
6159
}
60+
61+
func StartSpan(ctx context.Context, opts ...trace.SpanStartOption) (context.Context, trace.Span) {
62+
return trace.SpanFromContext(ctx).TracerProvider().Tracer("").Start(ctx, FuncNameSkip(1), opts...)
63+
}

‎coderd/httpapi/status_writer.go renamed to ‎coderd/tracing/status_writer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package httpapi
1+
package tracing
22

33
import (
44
"bufio"

‎coderd/httpapi/status_writer_test.go renamed to ‎coderd/tracing/status_writer_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package httpapi_test
1+
package tracing_test
22

33
import (
44
"bufio"
@@ -11,7 +11,7 @@ import (
1111
"github.com/stretchr/testify/require"
1212
"golang.org/x/xerrors"
1313

14-
"github.com/coder/coder/coderd/httpapi"
14+
"github.com/coder/coder/coderd/tracing"
1515
)
1616

1717
func TestStatusWriter(t *testing.T) {
@@ -22,7 +22,7 @@ func TestStatusWriter(t *testing.T) {
2222

2323
var (
2424
rec = httptest.NewRecorder()
25-
w = &httpapi.StatusWriter{ResponseWriter: rec}
25+
w = &tracing.StatusWriter{ResponseWriter: rec}
2626
)
2727

2828
w.WriteHeader(http.StatusOK)
@@ -36,7 +36,7 @@ func TestStatusWriter(t *testing.T) {
3636

3737
var (
3838
rec = httptest.NewRecorder()
39-
w = &httpapi.StatusWriter{ResponseWriter: rec}
39+
w = &tracing.StatusWriter{ResponseWriter: rec}
4040
code = http.StatusNotFound
4141
)
4242

@@ -52,7 +52,7 @@ func TestStatusWriter(t *testing.T) {
5252
t.Parallel()
5353
var (
5454
rec = httptest.NewRecorder()
55-
w = &httpapi.StatusWriter{ResponseWriter: rec}
55+
w = &tracing.StatusWriter{ResponseWriter: rec}
5656
body = []byte("hello")
5757
)
5858

@@ -70,7 +70,7 @@ func TestStatusWriter(t *testing.T) {
7070
t.Parallel()
7171
var (
7272
rec = httptest.NewRecorder()
73-
w = &httpapi.StatusWriter{ResponseWriter: rec}
73+
w = &tracing.StatusWriter{ResponseWriter: rec}
7474
body = []byte("hello")
7575
code = http.StatusInternalServerError
7676
)
@@ -88,7 +88,7 @@ func TestStatusWriter(t *testing.T) {
8888
t.Parallel()
8989
var (
9090
rec = httptest.NewRecorder()
91-
w = &httpapi.StatusWriter{ResponseWriter: rec}
91+
w = &tracing.StatusWriter{ResponseWriter: rec}
9292
// 8kb body.
9393
body = make([]byte, 8<<10)
9494
code = http.StatusInternalServerError
@@ -112,7 +112,7 @@ func TestStatusWriter(t *testing.T) {
112112
rec = httptest.NewRecorder()
113113
)
114114

115-
w := &httpapi.StatusWriter{ResponseWriter: hijacker{rec}}
115+
w := &tracing.StatusWriter{ResponseWriter: hijacker{rec}}
116116

117117
_, _, err := w.Hijack()
118118
require.Error(t, err)

‎coderd/tracing/util.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,16 @@ func FuncName() string {
1717
}
1818
return name
1919
}
20+
21+
func FuncNameSkip(skip int) string {
22+
fnpc, _, _, ok := runtime.Caller(1 + skip)
23+
if !ok {
24+
return ""
25+
}
26+
fn := runtime.FuncForPC(fnpc)
27+
name := fn.Name()
28+
if i := strings.LastIndex(name, "/"); i > 0 {
29+
name = name[i+1:]
30+
}
31+
return name
32+
}

‎provisionerd/provisionerd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func New(clientDialer Dialer, opts *Options) *Server {
7373
ctx, ctxCancel := context.WithCancel(context.Background())
7474
daemon := &Server{
7575
opts: opts,
76-
tracer: opts.Tracer.Tracer("provisionerd"),
76+
tracer: opts.Tracer.Tracer(""),
7777

7878
clientDialer: clientDialer,
7979

0 commit comments

Comments
 (0)