feat(annotations): allow creating multiple annotations for different …

…apps (#1562)

pkg/api/annotation.go

@@ -3,13 +3,15 @@ package api
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"strconv"
 	"time"
 
 	"github.com/pyroscope-io/pyroscope/pkg/model"
 	"github.com/pyroscope-io/pyroscope/pkg/server/httputils"
 	"github.com/pyroscope-io/pyroscope/pkg/util/attime"
+	"golang.org/x/sync/errgroup"
 )
 
 type AnnotationsService interface {
@@ -29,15 +31,15 @@ func NewAnnotationsHandler(svc AnnotationsService, httpUtils httputils.Utils) *A
 }
 
 type CreateParams struct {
-	AppName   string `json:"appName"`
-	Timestamp int64  `json:"timestamp"`
-	Content   string `json:"content"`
+	AppName   string   `json:"appName"`
+	AppNames  []string `json:"appNames"`
+	Timestamp int64    `json:"timestamp"`
+	Content   string   `json:"content"`
 }
 
 func (h *AnnotationsHandler) CreateAnnotation(w http.ResponseWriter, r *http.Request) {
-	var params CreateParams
-	if err := json.NewDecoder(r.Body).Decode(&params); err != nil {
-		h.httpUtils.WriteInternalServerError(r, w, err, "failed to unmarshal JSON")
+	params := h.validateAppNames(w, r)
+	if params == nil {
 		return
 	}
 
@@ -46,28 +48,91 @@ func (h *AnnotationsHandler) CreateAnnotation(w http.ResponseWriter, r *http.Req
 		timestamp = attime.Parse(strconv.FormatInt(params.Timestamp, 10))
 	}
 
-	annotation, err := h.svc.CreateAnnotation(r.Context(), model.CreateAnnotation{
-		AppName:   params.AppName,
-		Timestamp: timestamp,
-		Content:   params.Content,
-	})
-	if err != nil {
-		h.httpUtils.HandleError(r, w, err)
-		return
-	}
-
 	// TODO(eh-am): unify this with render.go
 	type annotationsResponse struct {
 		AppName   string `json:"appName"`
 		Content   string `json:"content"`
 		Timestamp int64  `json:"timestamp"`
 	}
-	annotationsResp := annotationsResponse{
-		AppName:   annotation.AppName,
-		Content:   annotation.Content,
-		Timestamp: annotation.Timestamp.Unix(),
+
+	createAnnotations := func(ctx context.Context, params *CreateParams) ([]annotationsResponse, error) {
+		g, ctx := errgroup.WithContext(ctx)
+
+		results := make([]annotationsResponse, len(params.AppNames))
+		for i, appName := range params.AppNames {
+			appName := appName
+			i := i
+			g.Go(func() error {
+				annotation, err := h.svc.CreateAnnotation(ctx, model.CreateAnnotation{
+					AppName:   appName,
+					Timestamp: timestamp,
+					Content:   params.Content,
+				})
+				if err != nil {
+					return err
+				}
+
+				results[i] = annotationsResponse{
+					AppName:   annotation.AppName,
+					Content:   annotation.Content,
+					Timestamp: annotation.Timestamp.Unix(),
+				}
+
+				return err
+			})
+		}
+
+		if err := g.Wait(); err != nil {
+			return nil, err
+		}
+		return results, nil
+	}
+
+	res, err := createAnnotations(r.Context(), params)
+	if err != nil {
+		h.httpUtils.HandleError(r, w, err)
+		return
 	}
 
 	w.WriteHeader(http.StatusCreated)
-	h.httpUtils.WriteResponseJSON(r, w, annotationsResp)
+	if len(res) == 1 {
+		h.httpUtils.WriteResponseJSON(r, w, res[0])
+		return
+	}
+
+	h.httpUtils.WriteResponseJSON(r, w, res)
+}
+
+// validateAppNames handles the different combinations between (`appName` and `appNames`)
+// in the failure case, it returns nil and serves an error
+// in the success case, it returns a `CreateParams` struct where `appNames` is ALWAYS populated with at least one appName
+func (h *AnnotationsHandler) validateAppNames(w http.ResponseWriter, r *http.Request) *CreateParams {
+	var params CreateParams
+
+	if err := json.NewDecoder(r.Body).Decode(&params); err != nil {
+		h.httpUtils.WriteInternalServerError(r, w, err, "failed to unmarshal JSON")
+		return nil
+	}
+
+	// handling `appName` and `appNames`
+	// 1. Both are set, is invalid
+	if params.AppName != "" && len(params.AppNames) > 0 {
+		h.httpUtils.HandleError(r, w, model.ValidationError{errors.New("only one of 'appName' and 'appNames' can be specified")})
+		return nil
+	}
+
+	// 2. None are set
+	if params.AppName == "" && len(params.AppNames) <= 0 {
+		h.httpUtils.HandleError(r, w, model.ValidationError{errors.New("at least one of 'appName' and 'appNames' needs to be specified")})
+		return nil
+	}
+
+	// 3. Only appName is set
+	if params.AppName != "" && len(params.AppNames) <= 0 {
+		params.AppNames = append(params.AppNames, params.AppName)
+		return &params
+	}
+
+	// 4. Only appNames is set
+	return &params
 }

pkg/api/annotation_test.go

@@ -94,6 +94,34 @@ var _ = Describe("AnnotationHandler", func() {
 			})
 		})
 
+		When("multiple appNames are passed", func() {
+			BeforeEach(func() {
+				svc = &mockService{
+					createAnnotationResponse: func(params model.CreateAnnotation) (*model.Annotation, error) {
+						return &model.Annotation{
+							AppName:   params.AppName,
+							Content:   "mycontent",
+							Timestamp: time.Unix(1662729099, 0),
+						}, nil
+					},
+				}
+
+				server = httptest.NewServer(newTestRouter(defaultUserCtx, router.Services{
+					Logger:             logrus.StandardLogger(),
+					AnnotationsService: svc,
+				}))
+			})
+
+			It("creates correctly", func() {
+				url := server.URL + "/annotations"
+
+				expectResponse(newRequest(http.MethodPost, url,
+					"annotation/create_multiple_request.json"),
+					"annotation/create_multiple_response.json",
+					http.StatusCreated)
+			})
+		})
+
 		When("fields are invalid", func() {
 			BeforeEach(func() {
 				svc = &mockService{
@@ -116,5 +144,51 @@ var _ = Describe("AnnotationHandler", func() {
 					http.StatusBadRequest)
 			})
 		})
+
+		When("both 'appName' and 'appNames' are passed", func() {
+			BeforeEach(func() {
+				svc = &mockService{
+					createAnnotationResponse: func(params model.CreateAnnotation) (*model.Annotation, error) {
+						return nil, nil
+					},
+				}
+
+				server = httptest.NewServer(newTestRouter(defaultUserCtx, router.Services{
+					Logger:             logrus.StandardLogger(),
+					AnnotationsService: svc,
+				}))
+			})
+			It("returns an error", func() {
+				url := server.URL + "/annotations"
+
+				expectResponse(newRequest(http.MethodPost, url,
+					"annotation/create_request_appName_appNames_error.json"),
+					"annotation/create_response_appName_appNames_error.json",
+					http.StatusBadRequest)
+			})
+		})
+
+		When("none of 'appName' and 'appNames' are passed", func() {
+			BeforeEach(func() {
+				svc = &mockService{
+					createAnnotationResponse: func(params model.CreateAnnotation) (*model.Annotation, error) {
+						return nil, nil
+					},
+				}
+
+				server = httptest.NewServer(newTestRouter(defaultUserCtx, router.Services{
+					Logger:             logrus.StandardLogger(),
+					AnnotationsService: svc,
+				}))
+			})
+			It("returns an error", func() {
+				url := server.URL + "/annotations"
+
+				expectResponse(newRequest(http.MethodPost, url,
+					"annotation/create_request_appName_appNames_empty_error.json"),
+					"annotation/create_response_appName_appNames_empty_error.json",
+					http.StatusBadRequest)
+			})
+		})
 	})
 })

pkg/api/testdata/annotation/create_multiple_request.json

@@ -0,0 +1,5 @@
+{
+  "appNames": ["myApp1.cpu", "myApp1.goroutines", "myApp1.alloc_objects"],
+  "content": "mycontent",
+  "timestamp": 1662729099
+}

pkg/api/testdata/annotation/create_multiple_response.json

@@ -0,0 +1,17 @@
+[
+  {
+    "appName": "myApp1.cpu",
+    "content": "mycontent",
+    "timestamp": 1662729099
+  },
+  {
+    "appName": "myApp1.goroutines",
+    "content": "mycontent",
+    "timestamp": 1662729099
+  },
+  {
+    "appName": "myApp1.alloc_objects",
+    "content": "mycontent",
+    "timestamp": 1662729099
+  }
+]

pkg/api/testdata/annotation/create_request_appName_appNames_empty_error.json

@@ -0,0 +1,4 @@
+{
+  "content": "mycontent",
+  "timestamp": 1662729099
+}

pkg/api/testdata/annotation/create_request_appName_appNames_error.json

@@ -0,0 +1,6 @@
+{
+  "appName": "myapp",
+  "appNames": ["myApp1.goroutines"],
+  "content": "mycontent",
+  "timestamp": 1662729099
+}

pkg/api/testdata/annotation/create_request_error.json

@@ -1 +1,3 @@
-{}
+{
+  "appName": "_"
+}

pkg/api/testdata/annotation/create_response_appName_appNames_empty_error.json

@@ -0,0 +1,3 @@
+{
+  "errors": ["at least one of 'appName' and 'appNames' needs to be specified"]
+}

pkg/api/testdata/annotation/create_response_appName_appNames_error.json

@@ -0,0 +1,3 @@
+{
+  "errors": ["only one of 'appName' and 'appNames' can be specified"]
+}