Skip to content

Commit 11859ff

Browse files
cedicedi
committed
Rework traces
1 parent 7d59235 commit 11859ff

13 files changed

+232
-190
lines changed

‎go.mod

+10-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/contrib/instrumentation/net/http/otelhttp v0.40.0
17+
go.opentelemetry.io/otel v1.14.0
1718
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.13.0
1819
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
20+
go.opentelemetry.io/otel/sdk v1.14.0
21+
go.opentelemetry.io/otel/trace v1.14.0
2122
golang.org/x/exp v0.0.0-20230304125523-9ff063c70017
2223
k8s.io/api v0.26.2
2324
k8s.io/apimachinery v0.26.2
2425
k8s.io/client-go v0.26.2
2526
sigs.k8s.io/controller-runtime v0.14.5
2627
)
2728

29+
require (
30+
github.com/felixge/httpsnoop v1.0.3 // indirect
31+
go.opentelemetry.io/otel/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,7 @@ 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
8187
go.opentelemetry.io/proto/otlp v0.19.0 // indirect
8288
go.uber.org/atomic v1.10.0 // indirect
8389
go.uber.org/multierr v1.9.0 // indirect

‎go.sum

+15-75
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

+26-15
Original file line numberDiff line numberDiff line change
@@ -36,28 +36,39 @@ 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(ct *gin.Context) {
40+
shortlinkName := ct.Param("shortlink")
41+
contentType := ct.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+
ctx := ct.Request.Context()
44+
span := trace.SpanFromContext(ctx)
4645

47-
bearerToken := c.Request.Header.Get("Authorization")
46+
// Check if the span was sampled and is recording the data
47+
if !span.IsRecording() {
48+
ctx, span = s.tracer.Start(ctx, "ShortlinkController.HandleCreateShortLink")
49+
defer span.End()
50+
}
51+
52+
span.SetAttributes(
53+
attribute.String("shortlink", shortlinkName),
54+
attribute.String("content_type", contentType),
55+
attribute.String("referrer", ct.Request.Referer()),
56+
)
57+
58+
bearerToken := ct.Request.Header.Get("Authorization")
4859
bearerToken = strings.TrimPrefix(bearerToken, "Bearer")
4960
bearerToken = strings.TrimPrefix(bearerToken, "token")
5061
if len(bearerToken) == 0 {
5162
err := fmt.Errorf("no credentials provided")
5263
span.RecordError(err)
53-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
64+
ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error())
5465
return
5566
}
5667

5768
githubUser, err := getGitHubUserInfo(ctx, bearerToken)
5869
if err != nil {
5970
span.RecordError(err)
60-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
71+
ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error())
6172
return
6273
}
6374

@@ -68,29 +79,29 @@ func (s *ShortlinkController) HandleCreateShortLink(c *gin.Context) {
6879
Spec: v1alpha1.ShortLinkSpec{},
6980
}
7081

71-
jsonData, err := io.ReadAll(c.Request.Body)
82+
jsonData, err := io.ReadAll(ct.Request.Body)
7283
if err != nil {
7384
observability.RecordError(span, s.log, err, "Failed to read request-body")
74-
ginReturnError(c, http.StatusInternalServerError, contentType, err.Error())
85+
ginReturnError(ct, http.StatusInternalServerError, contentType, err.Error())
7586
return
7687
}
7788

7889
if err := json.Unmarshal([]byte(jsonData), &shortlink.Spec); err != nil {
7990
observability.RecordError(span, s.log, err, "Failed to read spec-json")
80-
ginReturnError(c, http.StatusInternalServerError, contentType, err.Error())
91+
ginReturnError(ct, http.StatusInternalServerError, contentType, err.Error())
8192
return
8293
}
8394

8495
if err := s.authenticatedClient.Create(ctx, githubUser.Login, &shortlink); err != nil {
8596
observability.RecordError(span, s.log, err, "Failed to create ShortLink")
86-
ginReturnError(c, http.StatusInternalServerError, contentType, err.Error())
97+
ginReturnError(ct, http.StatusInternalServerError, contentType, err.Error())
8798
return
8899
}
89100

90101
if contentType == ContentTypeTextPlain {
91-
c.Data(http.StatusOK, contentType, []byte(fmt.Sprintf("%s: %s\n", shortlink.Name, shortlink.Spec.Target)))
102+
ct.Data(http.StatusOK, contentType, []byte(fmt.Sprintf("%s: %s\n", shortlink.Name, shortlink.Spec.Target)))
92103
} else if contentType == ContentTypeApplicationJSON {
93-
c.JSON(http.StatusOK, ShortLink{
104+
ct.JSON(http.StatusOK, ShortLink{
94105
Name: shortlink.Name,
95106
Spec: shortlink.Spec,
96107
Status: shortlink.Status,

‎pkg/controller/handle_delete_shortlink.go

+22-12
Original file line numberDiff line numberDiff line change
@@ -26,29 +26,39 @@ 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(ct *gin.Context) {
30+
shortlinkName := ct.Param("shortlink")
31+
contentType := ct.Request.Header.Get("accept")
3132

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

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()
36+
// Check if the span was sampled and is recording the data
37+
if !span.IsRecording() {
38+
ctx, span = s.tracer.Start(ctx, "ShortlinkController.HandleDeleteShortLink")
39+
defer span.End()
40+
}
41+
42+
span.SetAttributes(
43+
attribute.String("shortlink", shortlinkName),
44+
attribute.String("content_type", contentType),
45+
attribute.String("referrer", ct.Request.Referer()),
46+
)
3747

38-
bearerToken := c.Request.Header.Get("Authorization")
48+
bearerToken := ct.Request.Header.Get("Authorization")
3949
bearerToken = strings.TrimPrefix(bearerToken, "Bearer")
4050
bearerToken = strings.TrimPrefix(bearerToken, "token")
4151
if len(bearerToken) == 0 {
4252
err := fmt.Errorf("no credentials provided")
4353
span.RecordError(err)
44-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
54+
ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error())
4555
return
4656
}
4757

4858
githubUser, err := getGitHubUserInfo(ctx, bearerToken)
4959
if err != nil {
5060
span.RecordError(err)
51-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
61+
ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error())
5262
return
5363
}
5464

@@ -62,13 +72,13 @@ func (s *ShortlinkController) HandleDeleteShortLink(c *gin.Context) {
6272
statusCode = http.StatusNotFound
6373
}
6474

65-
ginReturnError(c, statusCode, contentType, err.Error())
75+
ginReturnError(ct, statusCode, contentType, err.Error())
6676
return
6777
}
6878

6979
// When shortlink was not found
7080
if shortlink == nil {
71-
ginReturnError(c, http.StatusNotFound, contentType, "Shortlink not found")
81+
ginReturnError(ct, http.StatusNotFound, contentType, "Shortlink not found")
7282
return
7383
}
7484

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

8292
observability.RecordError(span, s.log, err, "Failed to delete ShortLink")
8393

84-
ginReturnError(c, statusCode, contentType, err.Error())
94+
ginReturnError(ct, statusCode, contentType, err.Error())
8595
return
8696
}
8797
}

‎pkg/controller/handle_get_shortlink.go

+22-12
Original file line numberDiff line numberDiff line change
@@ -26,29 +26,39 @@ 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(ct *gin.Context) {
30+
shortlinkName := ct.Param("shortlink")
31+
contentType := ct.Request.Header.Get("accept")
3132

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

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()
36+
// Check if the span was sampled and is recording the data
37+
if !span.IsRecording() {
38+
ctx, span = s.tracer.Start(ctx, "ShortlinkController.HandleGetShortLink")
39+
defer span.End()
40+
}
41+
42+
span.SetAttributes(
43+
attribute.String("shortlink", shortlinkName),
44+
attribute.String("content_type", contentType),
45+
attribute.String("referrer", ct.Request.Referer()),
46+
)
3747

38-
bearerToken := c.Request.Header.Get("Authorization")
48+
bearerToken := ct.Request.Header.Get("Authorization")
3949
bearerToken = strings.TrimPrefix(bearerToken, "Bearer")
4050
bearerToken = strings.TrimPrefix(bearerToken, "token")
4151
if len(bearerToken) == 0 {
4252
err := fmt.Errorf("no credentials provided")
4353
span.RecordError(err)
44-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
54+
ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error())
4555
return
4656
}
4757

4858
githubUser, err := getGitHubUserInfo(ctx, bearerToken)
4959
if err != nil {
5060
span.RecordError(err)
51-
ginReturnError(c, http.StatusUnauthorized, contentType, err.Error())
61+
ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error())
5262
return
5363
}
5464

@@ -62,14 +72,14 @@ func (s *ShortlinkController) HandleGetShortLink(c *gin.Context) {
6272
statusCode = http.StatusNotFound
6373
}
6474

65-
ginReturnError(c, statusCode, contentType, err.Error())
75+
ginReturnError(ct, statusCode, contentType, err.Error())
6676
return
6777
}
6878

6979
if contentType == ContentTypeTextPlain {
70-
c.Data(http.StatusOK, contentType, []byte(shortlink.Spec.Target))
80+
ct.Data(http.StatusOK, contentType, []byte(shortlink.Spec.Target))
7181
} else if contentType == ContentTypeApplicationJSON {
72-
c.JSON(http.StatusOK, ShortLink{
82+
ct.JSON(http.StatusOK, ShortLink{
7383
Name: shortlink.Name,
7484
Spec: shortlink.Spec,
7585
Status: shortlink.Status,

0 commit comments

Comments
 (0)