Skip to content

Commit d863486

Browse files
cedicedi
committed
Rework traces
1 parent 7d59235 commit d863486

12 files changed

+195
-171
lines changed

‎go.mod

+11-4
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,24 @@ require (
1313
github.com/swaggo/gin-swagger v1.5.3
1414
github.com/swaggo/swag v1.8.10
1515
go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin v0.39.0
16-
go.opentelemetry.io/otel v1.13.0
16+
go.opentelemetry.io/otel v1.14.0
1717
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.13.0
1818
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.13.0
19-
go.opentelemetry.io/otel/sdk v1.13.0
20-
go.opentelemetry.io/otel/trace v1.13.0
19+
go.opentelemetry.io/otel/sdk v1.14.0
20+
go.opentelemetry.io/otel/trace v1.14.0
2121
golang.org/x/exp v0.0.0-20230304125523-9ff063c70017
2222
k8s.io/api v0.26.2
2323
k8s.io/apimachinery v0.26.2
2424
k8s.io/client-go v0.26.2
2525
sigs.k8s.io/controller-runtime v0.14.5
2626
)
2727

28+
require (
29+
go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.37.0 // indirect
30+
go.opentelemetry.io/otel/metric v0.37.0 // indirect
31+
go.opentelemetry.io/otel/sdk/metric v0.37.0 // indirect
32+
)
33+
2834
require (
2935
github.com/KyleBanks/depth v1.2.1 // indirect
3036
github.com/benbjohnson/clock v1.3.0 // indirect
@@ -77,7 +83,8 @@ require (
7783
github.com/spf13/pflag v1.0.5 // indirect
7884
github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
7985
github.com/ugorji/go/codec v1.2.9 // indirect
80-
go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.13.0 // indirect
86+
go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.14.0 // indirect
87+
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.37.0
8188
go.opentelemetry.io/proto/otlp v0.19.0 // indirect
8289
go.uber.org/atomic v1.10.0 // indirect
8390
go.uber.org/multierr v1.9.0 // indirect

‎go.sum

+16-66
Large diffs are not rendered by default.

‎pkg/client/authenticated_shortlink_client.go

+19-11
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import (
44
"context"
55

66
"github.com/cedi/urlshortener/api/v1alpha1"
7+
"github.com/cedi/urlshortener/pkg/model"
8+
79
"github.com/go-logr/logr"
10+
"github.com/pkg/errors"
11+
"go.opentelemetry.io/otel/attribute"
812
"go.opentelemetry.io/otel/trace"
9-
"golang.org/x/exp/slices"
1013
)
1114

1215
type ShortlinkClientAuth struct {
@@ -27,6 +30,8 @@ func (c *ShortlinkClientAuth) List(ct context.Context, username string) (*v1alph
2730
ctx, span := c.tracer.Start(ct, "ShortlinkClientAuth.List")
2831
defer span.End()
2932

33+
span.SetAttributes(attribute.String("username", username))
34+
3035
list, err := c.client.List(ctx)
3136
if err != nil {
3237
return nil, err
@@ -51,13 +56,15 @@ func (c *ShortlinkClientAuth) Get(ct context.Context, username string, name stri
5156
ctx, span := c.tracer.Start(ct, "ShortlinkClientAuth.Get")
5257
defer span.End()
5358

59+
span.SetAttributes(attribute.String("username", username))
60+
5461
shortLink, err := c.client.Get(ctx, name)
5562
if err != nil {
56-
return nil, err
63+
return nil, errors.Wrap(err, "Unable to get shortlink")
5764
}
5865

5966
if !shortLink.IsOwnedBy(username) {
60-
return nil, nil
67+
return nil, model.NewNotAllowedError(username, "delete", shortLink.Name)
6168
}
6269

6370
return shortLink, nil
@@ -67,21 +74,20 @@ func (c *ShortlinkClientAuth) Create(ct context.Context, username string, shortL
6774
ctx, span := c.tracer.Start(ct, "ShortlinkClientAuth.Create")
6875
defer span.End()
6976

70-
shortLink.Spec.Owner = username
77+
span.SetAttributes(attribute.String("username", username))
7178

79+
shortLink.Spec.Owner = username
7280
return c.client.Create(ctx, shortLink)
7381
}
7482

7583
func (c *ShortlinkClientAuth) Update(ct context.Context, username string, shortLink *v1alpha1.ShortLink) error {
7684
ctx, span := c.tracer.Start(ct, "ShortlinkClientAuth.Update")
7785
defer span.End()
7886

79-
// When someone updates a shortlink and removes himself as the owner
80-
// add him to the CoOwner
81-
if shortLink.Spec.Owner != username {
82-
if !slices.Contains(shortLink.Spec.CoOwners, username) {
83-
shortLink.Spec.CoOwners = append(shortLink.Spec.CoOwners, username)
84-
}
87+
span.SetAttributes(attribute.String("username", username))
88+
89+
if !shortLink.IsOwnedBy(username) {
90+
return model.NewNotAllowedError(username, "delete", shortLink.Name)
8591
}
8692

8793
if err := c.client.Update(ctx, shortLink); err != nil {
@@ -96,8 +102,10 @@ func (c *ShortlinkClientAuth) Delete(ct context.Context, username string, shortL
96102
ctx, span := c.tracer.Start(ct, "ShortlinkClientAuth.Update")
97103
defer span.End()
98104

105+
span.SetAttributes(attribute.String("username", username))
106+
99107
if !shortLink.IsOwnedBy(username) {
100-
return nil
108+
return model.NewNotAllowedError(username, "delete", shortLink.Name)
101109
}
102110

103111
return c.client.Delete(ctx, shortLink)

‎pkg/client/shortlink_client.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func (c *ShortlinkClient) ListNamespaced(ct context.Context, namespace string) (
104104
}
105105

106106
func (c *ShortlinkClient) Update(ct context.Context, shortlink *v1alpha1.ShortLink) error {
107-
ctx, span := c.tracer.Start(ct, "ShortlinkClient.Save", trace.WithAttributes(attribute.String("shortlink", shortlink.ObjectMeta.Name), attribute.String("namespace", shortlink.ObjectMeta.Namespace)))
107+
ctx, span := c.tracer.Start(ct, "ShortlinkClient.Update", trace.WithAttributes(attribute.String("shortlink", shortlink.ObjectMeta.Name), attribute.String("namespace", shortlink.ObjectMeta.Namespace)))
108108
defer span.End()
109109

110110
if err := c.client.Update(ctx, shortlink); err != nil {
@@ -116,7 +116,7 @@ func (c *ShortlinkClient) Update(ct context.Context, shortlink *v1alpha1.ShortLi
116116
}
117117

118118
func (c *ShortlinkClient) UpdateStatus(ct context.Context, shortlink *v1alpha1.ShortLink) error {
119-
ctx, span := c.tracer.Start(ct, "ShortlinkClient.SaveStatus", trace.WithAttributes(attribute.String("shortlink", shortlink.ObjectMeta.Name), attribute.String("namespace", shortlink.ObjectMeta.Namespace)))
119+
ctx, span := c.tracer.Start(ct, "ShortlinkClient.UpdateStatus", trace.WithAttributes(attribute.String("shortlink", shortlink.ObjectMeta.Name), attribute.String("namespace", shortlink.ObjectMeta.Namespace)))
120120
defer span.End()
121121

122122
err := c.client.Status().Update(ctx, shortlink)
@@ -128,7 +128,7 @@ func (c *ShortlinkClient) UpdateStatus(ct context.Context, shortlink *v1alpha1.S
128128
}
129129

130130
func (c *ShortlinkClient) IncrementInvocationCount(ct context.Context, shortlink *v1alpha1.ShortLink) error {
131-
ctx, span := c.tracer.Start(ct, "ShortlinkClient.SaveStatus", trace.WithAttributes(attribute.String("shortlink", shortlink.ObjectMeta.Name), attribute.String("namespace", shortlink.ObjectMeta.Namespace)))
131+
ctx, span := c.tracer.Start(ct, "ShortlinkClient.IncrementInvocationCount", trace.WithAttributes(attribute.String("shortlink", shortlink.ObjectMeta.Name), attribute.String("namespace", shortlink.ObjectMeta.Namespace)))
132132
defer span.End()
133133

134134
shortlink.Status.Count = shortlink.Status.Count + 1
@@ -141,7 +141,6 @@ func (c *ShortlinkClient) IncrementInvocationCount(ct context.Context, shortlink
141141
return nil
142142
}
143143

144-
// Delete deletes a Shortlink object
145144
func (c *ShortlinkClient) Delete(ct context.Context, shortlink *v1alpha1.ShortLink) error {
146145
ctx, span := c.tracer.Start(ct, "ShortlinkClient.Delete", trace.WithAttributes(attribute.String("name", shortlink.Name), attribute.String("namespace", shortlink.Namespace)))
147146
defer span.End()

‎pkg/controller/handle_create_shortlink.go

+19-15
Original file line numberDiff line numberDiff line change
@@ -36,28 +36,32 @@ import (
3636
// @Tags api/v1/
3737
// @Router /api/v1/shortlink/{shortlink} [post]
3838
// @Security bearerAuth
39-
func (s *ShortlinkController) HandleCreateShortLink(c *gin.Context) {
40-
shortlinkName := c.Param("shortlink")
41-
contentType := c.Request.Header.Get("accept")
39+
func (s *ShortlinkController) HandleCreateShortLink(ctx *gin.Context) {
40+
shortlinkName := ctx.Param("shortlink")
41+
contentType := ctx.Request.Header.Get("accept")
4242

43-
// Call the HTML method of the Context to render a template
44-
ctx, span := s.tracer.Start(c.Request.Context(), "ShortlinkController.HandleGetShortLink", trace.WithAttributes(attribute.String("shortlink", shortlinkName), attribute.String("accepted_content_type", contentType)))
45-
defer span.End()
43+
span := trace.SpanFromContext(ctx)
4644

47-
bearerToken := c.Request.Header.Get("Authorization")
45+
span.SetAttributes(
46+
attribute.String("shortlink", shortlinkName),
47+
attribute.String("content_type", contentType),
48+
attribute.String("referrer", ctx.Request.Referer()),
49+
)
50+
51+
bearerToken := ctx.Request.Header.Get("Authorization")
4852
bearerToken = strings.TrimPrefix(bearerToken, "Bearer")
4953
bearerToken = strings.TrimPrefix(bearerToken, "token")
5054
if len(bearerToken) == 0 {
5155
err := fmt.Errorf("no credentials provided")
5256
span.RecordError(err)
53-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
57+
ginReturnError(ctx, http.StatusUnauthorized, contentType, err.Error())
5458
return
5559
}
5660

5761
githubUser, err := getGitHubUserInfo(ctx, bearerToken)
5862
if err != nil {
5963
span.RecordError(err)
60-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
64+
ginReturnError(ctx, http.StatusUnauthorized, contentType, err.Error())
6165
return
6266
}
6367

@@ -68,29 +72,29 @@ func (s *ShortlinkController) HandleCreateShortLink(c *gin.Context) {
6872
Spec: v1alpha1.ShortLinkSpec{},
6973
}
7074

71-
jsonData, err := io.ReadAll(c.Request.Body)
75+
jsonData, err := io.ReadAll(ctx.Request.Body)
7276
if err != nil {
7377
observability.RecordError(span, s.log, err, "Failed to read request-body")
74-
ginReturnError(c, http.StatusInternalServerError, contentType, err.Error())
78+
ginReturnError(ctx, http.StatusInternalServerError, contentType, err.Error())
7579
return
7680
}
7781

7882
if err := json.Unmarshal([]byte(jsonData), &shortlink.Spec); err != nil {
7983
observability.RecordError(span, s.log, err, "Failed to read spec-json")
80-
ginReturnError(c, http.StatusInternalServerError, contentType, err.Error())
84+
ginReturnError(ctx, http.StatusInternalServerError, contentType, err.Error())
8185
return
8286
}
8387

8488
if err := s.authenticatedClient.Create(ctx, githubUser.Login, &shortlink); err != nil {
8589
observability.RecordError(span, s.log, err, "Failed to create ShortLink")
86-
ginReturnError(c, http.StatusInternalServerError, contentType, err.Error())
90+
ginReturnError(ctx, http.StatusInternalServerError, contentType, err.Error())
8791
return
8892
}
8993

9094
if contentType == ContentTypeTextPlain {
91-
c.Data(http.StatusOK, contentType, []byte(fmt.Sprintf("%s: %s\n", shortlink.Name, shortlink.Spec.Target)))
95+
ctx.Data(http.StatusOK, contentType, []byte(fmt.Sprintf("%s: %s\n", shortlink.Name, shortlink.Spec.Target)))
9296
} else if contentType == ContentTypeApplicationJSON {
93-
c.JSON(http.StatusOK, ShortLink{
97+
ctx.JSON(http.StatusOK, ShortLink{
9498
Name: shortlink.Name,
9599
Spec: shortlink.Spec,
96100
Status: shortlink.Status,

‎pkg/controller/handle_delete_shortlink.go

+16-12
Original file line numberDiff line numberDiff line change
@@ -26,29 +26,33 @@ import (
2626
// @Tags api/v1/
2727
// @Router /api/v1/shortlink/{shortlink} [delete]
2828
// @Security bearerAuth
29-
func (s *ShortlinkController) HandleDeleteShortLink(c *gin.Context) {
30-
shortlinkName := c.Param("shortlink")
29+
func (s *ShortlinkController) HandleDeleteShortLink(ctx *gin.Context) {
30+
shortlinkName := ctx.Param("shortlink")
3131

32-
contentType := c.Request.Header.Get("accept")
32+
contentType := ctx.Request.Header.Get("accept")
3333

34-
// Call the HTML method of the Context to render a template
35-
ctx, span := s.tracer.Start(c.Request.Context(), "ShortlinkController.HandleGetShortLink", trace.WithAttributes(attribute.String("shortlink", shortlinkName), attribute.String("accepted_content_type", contentType)))
36-
defer span.End()
34+
span := trace.SpanFromContext(ctx)
3735

38-
bearerToken := c.Request.Header.Get("Authorization")
36+
span.SetAttributes(
37+
attribute.String("shortlink", shortlinkName),
38+
attribute.String("content_type", contentType),
39+
attribute.String("referrer", ctx.Request.Referer()),
40+
)
41+
42+
bearerToken := ctx.Request.Header.Get("Authorization")
3943
bearerToken = strings.TrimPrefix(bearerToken, "Bearer")
4044
bearerToken = strings.TrimPrefix(bearerToken, "token")
4145
if len(bearerToken) == 0 {
4246
err := fmt.Errorf("no credentials provided")
4347
span.RecordError(err)
44-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
48+
ginReturnError(ctx, http.StatusUnauthorized, contentType, err.Error())
4549
return
4650
}
4751

4852
githubUser, err := getGitHubUserInfo(ctx, bearerToken)
4953
if err != nil {
5054
span.RecordError(err)
51-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
55+
ginReturnError(ctx, http.StatusUnauthorized, contentType, err.Error())
5256
return
5357
}
5458

@@ -62,13 +66,13 @@ func (s *ShortlinkController) HandleDeleteShortLink(c *gin.Context) {
6266
statusCode = http.StatusNotFound
6367
}
6468

65-
ginReturnError(c, statusCode, contentType, err.Error())
69+
ginReturnError(ctx, statusCode, contentType, err.Error())
6670
return
6771
}
6872

6973
// When shortlink was not found
7074
if shortlink == nil {
71-
ginReturnError(c, http.StatusNotFound, contentType, "Shortlink not found")
75+
ginReturnError(ctx, http.StatusNotFound, contentType, "Shortlink not found")
7276
return
7377
}
7478

@@ -81,7 +85,7 @@ func (s *ShortlinkController) HandleDeleteShortLink(c *gin.Context) {
8185

8286
observability.RecordError(span, s.log, err, "Failed to delete ShortLink")
8387

84-
ginReturnError(c, statusCode, contentType, err.Error())
88+
ginReturnError(ctx, statusCode, contentType, err.Error())
8589
return
8690
}
8791
}

‎pkg/controller/handle_get_shortlink.go

+15-12
Original file line numberDiff line numberDiff line change
@@ -26,29 +26,32 @@ import (
2626
// @Tags api/v1/
2727
// @Router /api/v1/shortlink/{shortlink} [get]
2828
// @Security bearerAuth
29-
func (s *ShortlinkController) HandleGetShortLink(c *gin.Context) {
30-
shortlinkName := c.Param("shortlink")
29+
func (s *ShortlinkController) HandleGetShortLink(ctx *gin.Context) {
30+
shortlinkName := ctx.Param("shortlink")
31+
contentType := ctx.Request.Header.Get("accept")
3132

32-
contentType := c.Request.Header.Get("accept")
33+
span := trace.SpanFromContext(ctx)
3334

34-
// Call the HTML method of the Context to render a template
35-
ctx, span := s.tracer.Start(c.Request.Context(), "ShortlinkController.HandleGetShortLink", trace.WithAttributes(attribute.String("shortlink", shortlinkName), attribute.String("accepted_content_type", contentType)))
36-
defer span.End()
35+
span.SetAttributes(
36+
attribute.String("shortlink", shortlinkName),
37+
attribute.String("content_type", contentType),
38+
attribute.String("referrer", ctx.Request.Referer()),
39+
)
3740

38-
bearerToken := c.Request.Header.Get("Authorization")
41+
bearerToken := ctx.Request.Header.Get("Authorization")
3942
bearerToken = strings.TrimPrefix(bearerToken, "Bearer")
4043
bearerToken = strings.TrimPrefix(bearerToken, "token")
4144
if len(bearerToken) == 0 {
4245
err := fmt.Errorf("no credentials provided")
4346
span.RecordError(err)
44-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
47+
ginReturnError(ctx, http.StatusUnauthorized, contentType, err.Error())
4548
return
4649
}
4750

4851
githubUser, err := getGitHubUserInfo(ctx, bearerToken)
4952
if err != nil {
5053
span.RecordError(err)
51-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
54+
ginReturnError(ctx, http.StatusUnauthorized, contentType, err.Error())
5255
return
5356
}
5457

@@ -62,14 +65,14 @@ func (s *ShortlinkController) HandleGetShortLink(c *gin.Context) {
6265
statusCode = http.StatusNotFound
6366
}
6467

65-
ginReturnError(c, statusCode, contentType, err.Error())
68+
ginReturnError(ctx, statusCode, contentType, err.Error())
6669
return
6770
}
6871

6972
if contentType == ContentTypeTextPlain {
70-
c.Data(http.StatusOK, contentType, []byte(shortlink.Spec.Target))
73+
ctx.Data(http.StatusOK, contentType, []byte(shortlink.Spec.Target))
7174
} else if contentType == ContentTypeApplicationJSON {
72-
c.JSON(http.StatusOK, ShortLink{
75+
ctx.JSON(http.StatusOK, ShortLink{
7376
Name: shortlink.Name,
7477
Spec: shortlink.Spec,
7578
Status: shortlink.Status,

0 commit comments

Comments
 (0)