Browse Source

Annotations: use a single row to represent a region (#17673)

* SQLite migrations

* cleanup

* migrate end times

* switch to update with a query

* real migration

* anno migrations

* remove old docs

* set isRegion from time changes

* use <> for is not

* add comment and fix index decleration

* single validation place

* add test

* fix test

* add upgrading docs

* use AnnotationEvent

* fix import

* remove regionId from typescript
Ryan McKinley 6 years ago
parent
commit
6335509a23

+ 3 - 32
docs/sources/http_api/annotations.md

@@ -56,9 +56,9 @@ Content-Type: application/json
         "newState": "",
         "newState": "",
         "prevState": "",
         "prevState": "",
         "time": 1507266395000,
         "time": 1507266395000,
+        "timeEnd": 1507266395000,
         "text": "test",
         "text": "test",
         "metric": "",
         "metric": "",
-        "regionId": 1123,
         "type": "event",
         "type": "event",
         "tags": [
         "tags": [
             "tag1",
             "tag1",
@@ -78,7 +78,6 @@ Content-Type: application/json
         "time": 1507265111000,
         "time": 1507265111000,
         "text": "test",
         "text": "test",
         "metric": "",
         "metric": "",
-        "regionId": 1123,
         "type": "event",
         "type": "event",
         "tags": [
         "tags": [
             "tag1",
             "tag1",
@@ -106,7 +105,6 @@ Content-Type: application/json
   "dashboardId":468,
   "dashboardId":468,
   "panelId":1,
   "panelId":1,
   "time":1507037197339,
   "time":1507037197339,
-  "isRegion":true,
   "timeEnd":1507180805056,
   "timeEnd":1507180805056,
   "tags":["tag1","tag2"],
   "tags":["tag1","tag2"],
   "text":"Annotation Description"
   "text":"Annotation Description"
@@ -176,7 +174,6 @@ Content-Type: application/json
 
 
 {
 {
   "time":1507037197339,
   "time":1507037197339,
-  "isRegion":true,
   "timeEnd":1507180805056,
   "timeEnd":1507180805056,
   "text":"Annotation Description",
   "text":"Annotation Description",
   "tags":["tag3","tag4","tag5"]
   "tags":["tag3","tag4","tag5"]
@@ -201,7 +198,7 @@ Content-Type: application/json
 
 
 Updates one or more properties of an annotation that matches the specified id.
 Updates one or more properties of an annotation that matches the specified id.
 
 
-This operation currently supports updating of the `text`, `tags`, `time` and `timeEnd` properties. It does not handle updating of the `isRegion` and `regionId` properties. To make an annotation regional or vice versa, consider using the [Update Annotation](#update-annotation) operation.
+This operation currently supports updating of the `text`, `tags`, `time` and `timeEnd` properties. 
 
 
 **Example Request**:
 **Example Request**:
 
 
@@ -252,30 +249,4 @@ Content-Type: application/json
 {
 {
     "message":"Annotation deleted"
     "message":"Annotation deleted"
 }
 }
-```
-
-## Delete Annotation By RegionId
-
-`DELETE /api/annotations/region/:id`
-
-Deletes the annotation that matches the specified region id. A region is an annotation that covers a timerange and has a start and end time. In the Grafana database, this is a stored as two annotations connected by a region id.
-
-**Example Request**:
-
-```http
-DELETE /api/annotations/region/1 HTTP/1.1
-Accept: application/json
-Content-Type: application/json
-Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
-```
-
-**Example Response**:
-
-```http
-HTTP/1.1 200
-Content-Type: application/json
-
-{
-    "message":"Annotation region deleted"
-}
-```
+```

+ 6 - 0
docs/sources/installation/upgrading.md

@@ -134,6 +134,12 @@ If you're using systemd and have a large amount of annotations consider temporar
 If you have text panels with script tags they will no longer work due to a new setting that per default disallow unsanitized HTML.
 If you have text panels with script tags they will no longer work due to a new setting that per default disallow unsanitized HTML.
 Read more [here](/installation/configuration/#disable-sanitize-html) about this new setting.
 Read more [here](/installation/configuration/#disable-sanitize-html) about this new setting.
 
 
+
+## Upgrading to v6.4
+
+One of the database migrations included in this release will merge multiple rows used to represent an annotation range into a single row.  If you have a large number of region annotations the database migration may take a long time to complete.  See [Upgrading to v5.2](#upgrading-to-v5-2) for tips on how to manage this process.
+
+
 ### Authentication and security
 ### Authentication and security
 
 
 If your using Grafana's builtin, LDAP (without Auth Proxy) or OAuth authentication all users will be required to login upon the next visit after the upgrade.
 If your using Grafana's builtin, LDAP (without Auth Proxy) or OAuth authentication all users will be required to login upon the next visit after the upgrade.

+ 3 - 1
packages/grafana-data/src/types/data.ts

@@ -70,7 +70,6 @@ export interface AnnotationEvent {
   dashboardId?: number;
   dashboardId?: number;
   panelId?: number;
   panelId?: number;
   userId?: number;
   userId?: number;
-  regionId?: number;
   login?: string;
   login?: string;
   email?: string;
   email?: string;
   avatarUrl?: string;
   avatarUrl?: string;
@@ -81,4 +80,7 @@ export interface AnnotationEvent {
   text?: string;
   text?: string;
   type?: string;
   type?: string;
   tags?: string[];
   tags?: string[];
+
+  // Currently used to merge annotations from alerts and dashboard
+  source?: any; // source.type === 'dashboard'
 }
 }

+ 13 - 84
pkg/api/annotations.go

@@ -4,7 +4,6 @@ import (
 	"strings"
 	"strings"
 
 
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/api/dtos"
-	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/annotations"
 	"github.com/grafana/grafana/pkg/services/annotations"
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/guardian"
@@ -69,6 +68,7 @@ func PostAnnotation(c *m.ReqContext, cmd dtos.PostAnnotationsCmd) Response {
 		DashboardId: cmd.DashboardId,
 		DashboardId: cmd.DashboardId,
 		PanelId:     cmd.PanelId,
 		PanelId:     cmd.PanelId,
 		Epoch:       cmd.Time,
 		Epoch:       cmd.Time,
+		EpochEnd:    cmd.TimeEnd,
 		Text:        cmd.Text,
 		Text:        cmd.Text,
 		Data:        cmd.Data,
 		Data:        cmd.Data,
 		Tags:        cmd.Tags,
 		Tags:        cmd.Tags,
@@ -80,32 +80,6 @@ func PostAnnotation(c *m.ReqContext, cmd dtos.PostAnnotationsCmd) Response {
 
 
 	startID := item.Id
 	startID := item.Id
 
 
-	// handle regions
-	if cmd.IsRegion {
-		item.RegionId = startID
-
-		if item.Data == nil {
-			item.Data = simplejson.New()
-		}
-
-		if err := repo.Update(&item); err != nil {
-			return Error(500, "Failed set regionId on annotation", err)
-		}
-
-		item.Id = 0
-		item.Epoch = cmd.TimeEnd
-
-		if err := repo.Save(&item); err != nil {
-			return Error(500, "Failed save annotation for region end time", err)
-		}
-
-		return JSON(200, util.DynMap{
-			"message": "Annotation added",
-			"id":      startID,
-			"endId":   item.Id,
-		})
-	}
-
 	return JSON(200, util.DynMap{
 	return JSON(200, util.DynMap{
 		"message": "Annotation added",
 		"message": "Annotation added",
 		"id":      startID,
 		"id":      startID,
@@ -181,32 +155,19 @@ func UpdateAnnotation(c *m.ReqContext, cmd dtos.UpdateAnnotationsCmd) Response {
 	}
 	}
 
 
 	item := annotations.Item{
 	item := annotations.Item{
-		OrgId:  c.OrgId,
-		UserId: c.UserId,
-		Id:     annotationID,
-		Epoch:  cmd.Time,
-		Text:   cmd.Text,
-		Tags:   cmd.Tags,
+		OrgId:    c.OrgId,
+		UserId:   c.UserId,
+		Id:       annotationID,
+		Epoch:    cmd.Time,
+		EpochEnd: cmd.TimeEnd,
+		Text:     cmd.Text,
+		Tags:     cmd.Tags,
 	}
 	}
 
 
 	if err := repo.Update(&item); err != nil {
 	if err := repo.Update(&item); err != nil {
 		return Error(500, "Failed to update annotation", err)
 		return Error(500, "Failed to update annotation", err)
 	}
 	}
 
 
-	if cmd.IsRegion {
-		itemRight := item
-		itemRight.RegionId = item.Id
-		itemRight.Epoch = cmd.TimeEnd
-
-		// We don't know id of region right event, so set it to 0 and find then using query like
-		// ... WHERE region_id = <item.RegionId> AND id != <item.RegionId> ...
-		itemRight.Id = 0
-
-		if err := repo.Update(&itemRight); err != nil {
-			return Error(500, "Failed to update annotation for region end time", err)
-		}
-	}
-
 	return Success("Annotation updated")
 	return Success("Annotation updated")
 }
 }
 
 
@@ -230,9 +191,9 @@ func PatchAnnotation(c *m.ReqContext, cmd dtos.PatchAnnotationsCmd) Response {
 		UserId:   c.UserId,
 		UserId:   c.UserId,
 		Id:       annotationID,
 		Id:       annotationID,
 		Epoch:    items[0].Time,
 		Epoch:    items[0].Time,
+		EpochEnd: items[0].TimeEnd,
 		Text:     items[0].Text,
 		Text:     items[0].Text,
 		Tags:     items[0].Tags,
 		Tags:     items[0].Tags,
-		RegionId: items[0].RegionId,
 	}
 	}
 
 
 	if cmd.Tags != nil {
 	if cmd.Tags != nil {
@@ -247,23 +208,12 @@ func PatchAnnotation(c *m.ReqContext, cmd dtos.PatchAnnotationsCmd) Response {
 		existing.Epoch = cmd.Time
 		existing.Epoch = cmd.Time
 	}
 	}
 
 
-	if err := repo.Update(&existing); err != nil {
-		return Error(500, "Failed to update annotation", err)
+	if cmd.TimeEnd > 0 && cmd.TimeEnd != existing.EpochEnd {
+		existing.EpochEnd = cmd.TimeEnd
 	}
 	}
 
 
-	// Update region end time if provided
-	if existing.RegionId != 0 && cmd.TimeEnd > 0 {
-		itemRight := existing
-		itemRight.RegionId = existing.Id
-		itemRight.Epoch = cmd.TimeEnd
-
-		// We don't know id of region right event, so set it to 0 and find then using query like
-		// ... WHERE region_id = <item.RegionId> AND id != <item.RegionId> ...
-		itemRight.Id = 0
-
-		if err := repo.Update(&itemRight); err != nil {
-			return Error(500, "Failed to update annotation for region end time", err)
-		}
+	if err := repo.Update(&existing); err != nil {
+		return Error(500, "Failed to update annotation", err)
 	}
 	}
 
 
 	return Success("Annotation patched")
 	return Success("Annotation patched")
@@ -275,7 +225,6 @@ func DeleteAnnotations(c *m.ReqContext, cmd dtos.DeleteAnnotationsCmd) Response
 	err := repo.Delete(&annotations.DeleteParams{
 	err := repo.Delete(&annotations.DeleteParams{
 		OrgId:       c.OrgId,
 		OrgId:       c.OrgId,
 		Id:          cmd.AnnotationId,
 		Id:          cmd.AnnotationId,
-		RegionId:    cmd.RegionId,
 		DashboardId: cmd.DashboardId,
 		DashboardId: cmd.DashboardId,
 		PanelId:     cmd.PanelId,
 		PanelId:     cmd.PanelId,
 	})
 	})
@@ -307,26 +256,6 @@ func DeleteAnnotationByID(c *m.ReqContext) Response {
 	return Success("Annotation deleted")
 	return Success("Annotation deleted")
 }
 }
 
 
-func DeleteAnnotationRegion(c *m.ReqContext) Response {
-	repo := annotations.GetRepository()
-	regionID := c.ParamsInt64(":regionId")
-
-	if resp := canSave(c, repo, regionID); resp != nil {
-		return resp
-	}
-
-	err := repo.Delete(&annotations.DeleteParams{
-		OrgId:    c.OrgId,
-		RegionId: regionID,
-	})
-
-	if err != nil {
-		return Error(500, "Failed to delete annotation region", err)
-	}
-
-	return Success("Annotation region deleted")
-}
-
 func canSaveByDashboardID(c *m.ReqContext, dashboardID int64) (bool, error) {
 func canSaveByDashboardID(c *m.ReqContext, dashboardID int64) (bool, error) {
 	if dashboardID == 0 && !c.SignedInUser.HasRole(m.ROLE_EDITOR) {
 	if dashboardID == 0 && !c.SignedInUser.HasRole(m.ROLE_EDITOR) {
 		return false, nil
 		return false, nil

+ 10 - 38
pkg/api/annotations_test.go

@@ -14,17 +14,15 @@ import (
 func TestAnnotationsApiEndpoint(t *testing.T) {
 func TestAnnotationsApiEndpoint(t *testing.T) {
 	Convey("Given an annotation without a dashboard id", t, func() {
 	Convey("Given an annotation without a dashboard id", t, func() {
 		cmd := dtos.PostAnnotationsCmd{
 		cmd := dtos.PostAnnotationsCmd{
-			Time:     1000,
-			Text:     "annotation text",
-			Tags:     []string{"tag1", "tag2"},
-			IsRegion: false,
+			Time: 1000,
+			Text: "annotation text",
+			Tags: []string{"tag1", "tag2"},
 		}
 		}
 
 
 		updateCmd := dtos.UpdateAnnotationsCmd{
 		updateCmd := dtos.UpdateAnnotationsCmd{
-			Time:     1000,
-			Text:     "annotation text",
-			Tags:     []string{"tag1", "tag2"},
-			IsRegion: false,
+			Time: 1000,
+			Text: "annotation text",
+			Tags: []string{"tag1", "tag2"},
 		}
 		}
 
 
 		patchCmd := dtos.PatchAnnotationsCmd{
 		patchCmd := dtos.PatchAnnotationsCmd{
@@ -56,12 +54,6 @@ func TestAnnotationsApiEndpoint(t *testing.T) {
 					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
 					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
 					So(sc.resp.Code, ShouldEqual, 403)
 					So(sc.resp.Code, ShouldEqual, 403)
 				})
 				})
-
-				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/annotations/region/1", "/api/annotations/region/:regionId", role, func(sc *scenarioContext) {
-					sc.handlerFunc = DeleteAnnotationRegion
-					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
-					So(sc.resp.Code, ShouldEqual, 403)
-				})
 			})
 			})
 		})
 		})
 
 
@@ -88,12 +80,6 @@ func TestAnnotationsApiEndpoint(t *testing.T) {
 					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
 					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
 					So(sc.resp.Code, ShouldEqual, 200)
 					So(sc.resp.Code, ShouldEqual, 200)
 				})
 				})
-
-				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/annotations/region/1", "/api/annotations/region/:regionId", role, func(sc *scenarioContext) {
-					sc.handlerFunc = DeleteAnnotationRegion
-					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
-					So(sc.resp.Code, ShouldEqual, 200)
-				})
 			})
 			})
 		})
 		})
 	})
 	})
@@ -103,17 +89,15 @@ func TestAnnotationsApiEndpoint(t *testing.T) {
 			Time:        1000,
 			Time:        1000,
 			Text:        "annotation text",
 			Text:        "annotation text",
 			Tags:        []string{"tag1", "tag2"},
 			Tags:        []string{"tag1", "tag2"},
-			IsRegion:    false,
 			DashboardId: 1,
 			DashboardId: 1,
 			PanelId:     1,
 			PanelId:     1,
 		}
 		}
 
 
 		updateCmd := dtos.UpdateAnnotationsCmd{
 		updateCmd := dtos.UpdateAnnotationsCmd{
-			Time:     1000,
-			Text:     "annotation text",
-			Tags:     []string{"tag1", "tag2"},
-			IsRegion: false,
-			Id:       1,
+			Time: 1000,
+			Text: "annotation text",
+			Tags: []string{"tag1", "tag2"},
+			Id:   1,
 		}
 		}
 
 
 		patchCmd := dtos.PatchAnnotationsCmd{
 		patchCmd := dtos.PatchAnnotationsCmd{
@@ -169,12 +153,6 @@ func TestAnnotationsApiEndpoint(t *testing.T) {
 					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
 					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
 					So(sc.resp.Code, ShouldEqual, 403)
 					So(sc.resp.Code, ShouldEqual, 403)
 				})
 				})
-
-				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/annotations/region/1", "/api/annotations/region/:regionId", role, func(sc *scenarioContext) {
-					sc.handlerFunc = DeleteAnnotationRegion
-					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
-					So(sc.resp.Code, ShouldEqual, 403)
-				})
 			})
 			})
 		})
 		})
 
 
@@ -201,12 +179,6 @@ func TestAnnotationsApiEndpoint(t *testing.T) {
 					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
 					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
 					So(sc.resp.Code, ShouldEqual, 200)
 					So(sc.resp.Code, ShouldEqual, 200)
 				})
 				})
-
-				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/annotations/region/1", "/api/annotations/region/:regionId", role, func(sc *scenarioContext) {
-					sc.handlerFunc = DeleteAnnotationRegion
-					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec()
-					So(sc.resp.Code, ShouldEqual, 200)
-				})
 			})
 			})
 		})
 		})
 
 

+ 0 - 1
pkg/api/api.go

@@ -371,7 +371,6 @@ func (hs *HTTPServer) registerRoutes() {
 			annotationsRoute.Delete("/:annotationId", Wrap(DeleteAnnotationByID))
 			annotationsRoute.Delete("/:annotationId", Wrap(DeleteAnnotationByID))
 			annotationsRoute.Put("/:annotationId", bind(dtos.UpdateAnnotationsCmd{}), Wrap(UpdateAnnotation))
 			annotationsRoute.Put("/:annotationId", bind(dtos.UpdateAnnotationsCmd{}), Wrap(UpdateAnnotation))
 			annotationsRoute.Patch("/:annotationId", bind(dtos.PatchAnnotationsCmd{}), Wrap(PatchAnnotation))
 			annotationsRoute.Patch("/:annotationId", bind(dtos.PatchAnnotationsCmd{}), Wrap(PatchAnnotation))
-			annotationsRoute.Delete("/region/:regionId", Wrap(DeleteAnnotationRegion))
 			annotationsRoute.Post("/graphite", reqEditorRole, bind(dtos.PostGraphiteAnnotationsCmd{}), Wrap(PostGraphiteAnnotation))
 			annotationsRoute.Post("/graphite", reqEditorRole, bind(dtos.PostGraphiteAnnotationsCmd{}), Wrap(PostGraphiteAnnotation))
 		})
 		})
 
 

+ 7 - 10
pkg/api/dtos/annotations.go

@@ -6,28 +6,26 @@ type PostAnnotationsCmd struct {
 	DashboardId int64            `json:"dashboardId"`
 	DashboardId int64            `json:"dashboardId"`
 	PanelId     int64            `json:"panelId"`
 	PanelId     int64            `json:"panelId"`
 	Time        int64            `json:"time"`
 	Time        int64            `json:"time"`
+	TimeEnd     int64            `json:"timeEnd,omitempty"` // Optional
 	Text        string           `json:"text"`
 	Text        string           `json:"text"`
 	Tags        []string         `json:"tags"`
 	Tags        []string         `json:"tags"`
 	Data        *simplejson.Json `json:"data"`
 	Data        *simplejson.Json `json:"data"`
-	IsRegion    bool             `json:"isRegion"`
-	TimeEnd     int64            `json:"timeEnd"`
 }
 }
 
 
 type UpdateAnnotationsCmd struct {
 type UpdateAnnotationsCmd struct {
-	Id       int64    `json:"id"`
-	Time     int64    `json:"time"`
-	Text     string   `json:"text"`
-	Tags     []string `json:"tags"`
-	IsRegion bool     `json:"isRegion"`
-	TimeEnd  int64    `json:"timeEnd"`
+	Id      int64    `json:"id"`
+	Time    int64    `json:"time"`
+	TimeEnd int64    `json:"timeEnd,omitempty"` // Optional
+	Text    string   `json:"text"`
+	Tags    []string `json:"tags"`
 }
 }
 
 
 type PatchAnnotationsCmd struct {
 type PatchAnnotationsCmd struct {
 	Id      int64    `json:"id"`
 	Id      int64    `json:"id"`
 	Time    int64    `json:"time"`
 	Time    int64    `json:"time"`
+	TimeEnd int64    `json:"timeEnd,omitempty"` // Optional
 	Text    string   `json:"text"`
 	Text    string   `json:"text"`
 	Tags    []string `json:"tags"`
 	Tags    []string `json:"tags"`
-	TimeEnd int64    `json:"timeEnd"`
 }
 }
 
 
 type DeleteAnnotationsCmd struct {
 type DeleteAnnotationsCmd struct {
@@ -35,7 +33,6 @@ type DeleteAnnotationsCmd struct {
 	DashboardId  int64 `json:"dashboardId"`
 	DashboardId  int64 `json:"dashboardId"`
 	PanelId      int64 `json:"panelId"`
 	PanelId      int64 `json:"panelId"`
 	AnnotationId int64 `json:"annotationId"`
 	AnnotationId int64 `json:"annotationId"`
-	RegionId     int64 `json:"regionId"`
 }
 }
 
 
 type PostGraphiteAnnotationsCmd struct {
 type PostGraphiteAnnotationsCmd struct {

+ 2 - 4
pkg/services/annotations/annotations.go

@@ -18,7 +18,6 @@ type ItemQuery struct {
 	DashboardId  int64    `json:"dashboardId"`
 	DashboardId  int64    `json:"dashboardId"`
 	PanelId      int64    `json:"panelId"`
 	PanelId      int64    `json:"panelId"`
 	AnnotationId int64    `json:"annotationId"`
 	AnnotationId int64    `json:"annotationId"`
-	RegionId     int64    `json:"regionId"`
 	Tags         []string `json:"tags"`
 	Tags         []string `json:"tags"`
 	Type         string   `json:"type"`
 	Type         string   `json:"type"`
 	MatchAny     bool     `json:"matchAny"`
 	MatchAny     bool     `json:"matchAny"`
@@ -41,7 +40,6 @@ type DeleteParams struct {
 	AlertId     int64
 	AlertId     int64
 	DashboardId int64
 	DashboardId int64
 	PanelId     int64
 	PanelId     int64
-	RegionId    int64
 }
 }
 
 
 var repositoryInstance Repository
 var repositoryInstance Repository
@@ -60,12 +58,12 @@ type Item struct {
 	UserId      int64            `json:"userId"`
 	UserId      int64            `json:"userId"`
 	DashboardId int64            `json:"dashboardId"`
 	DashboardId int64            `json:"dashboardId"`
 	PanelId     int64            `json:"panelId"`
 	PanelId     int64            `json:"panelId"`
-	RegionId    int64            `json:"regionId"`
 	Text        string           `json:"text"`
 	Text        string           `json:"text"`
 	AlertId     int64            `json:"alertId"`
 	AlertId     int64            `json:"alertId"`
 	PrevState   string           `json:"prevState"`
 	PrevState   string           `json:"prevState"`
 	NewState    string           `json:"newState"`
 	NewState    string           `json:"newState"`
 	Epoch       int64            `json:"epoch"`
 	Epoch       int64            `json:"epoch"`
+	EpochEnd    int64            `json:"epochEnd"`
 	Created     int64            `json:"created"`
 	Created     int64            `json:"created"`
 	Updated     int64            `json:"updated"`
 	Updated     int64            `json:"updated"`
 	Tags        []string         `json:"tags"`
 	Tags        []string         `json:"tags"`
@@ -88,8 +86,8 @@ type ItemDTO struct {
 	Created     int64            `json:"created"`
 	Created     int64            `json:"created"`
 	Updated     int64            `json:"updated"`
 	Updated     int64            `json:"updated"`
 	Time        int64            `json:"time"`
 	Time        int64            `json:"time"`
+	TimeEnd     int64            `json:"timeEnd"`
 	Text        string           `json:"text"`
 	Text        string           `json:"text"`
-	RegionId    int64            `json:"regionId"`
 	Tags        []string         `json:"tags"`
 	Tags        []string         `json:"tags"`
 	Login       string           `json:"login"`
 	Login       string           `json:"login"`
 	Email       string           `json:"email"`
 	Email       string           `json:"email"`

+ 38 - 23
pkg/services/sqlstore/annotation.go

@@ -11,6 +11,25 @@ import (
 	"github.com/grafana/grafana/pkg/services/annotations"
 	"github.com/grafana/grafana/pkg/services/annotations"
 )
 )
 
 
+// Update the item so that EpochEnd >= Epoch
+func validateTimeRange(item *annotations.Item) error {
+	if item.EpochEnd == 0 {
+		if item.Epoch == 0 {
+			return errors.New("Missing Time Range")
+		}
+		item.EpochEnd = item.Epoch
+	}
+	if item.Epoch == 0 {
+		item.Epoch = item.EpochEnd
+	}
+	if item.EpochEnd < item.Epoch {
+		tmp := item.Epoch
+		item.Epoch = item.EpochEnd
+		item.EpochEnd = tmp
+	}
+	return nil
+}
+
 type SqlAnnotationRepo struct {
 type SqlAnnotationRepo struct {
 }
 }
 
 
@@ -23,6 +42,9 @@ func (r *SqlAnnotationRepo) Save(item *annotations.Item) error {
 		if item.Epoch == 0 {
 		if item.Epoch == 0 {
 			item.Epoch = item.Created
 			item.Epoch = item.Created
 		}
 		}
+		if err := validateTimeRange(item); err != nil {
+			return err
+		}
 
 
 		if _, err := sess.Table("annotation").Insert(item); err != nil {
 		if _, err := sess.Table("annotation").Insert(item); err != nil {
 			return err
 			return err
@@ -52,12 +74,7 @@ func (r *SqlAnnotationRepo) Update(item *annotations.Item) error {
 		)
 		)
 		existing := new(annotations.Item)
 		existing := new(annotations.Item)
 
 
-		if item.Id == 0 && item.RegionId != 0 {
-			// Update region end time
-			isExist, err = sess.Table("annotation").Where("region_id=? AND id!=? AND org_id=?", item.RegionId, item.RegionId, item.OrgId).Get(existing)
-		} else {
-			isExist, err = sess.Table("annotation").Where("id=? AND org_id=?", item.Id, item.OrgId).Get(existing)
-		}
+		isExist, err = sess.Table("annotation").Where("id=? AND org_id=?", item.Id, item.OrgId).Get(existing)
 
 
 		if err != nil {
 		if err != nil {
 			return err
 			return err
@@ -67,10 +84,17 @@ func (r *SqlAnnotationRepo) Update(item *annotations.Item) error {
 		}
 		}
 
 
 		existing.Updated = time.Now().UnixNano() / int64(time.Millisecond)
 		existing.Updated = time.Now().UnixNano() / int64(time.Millisecond)
-		existing.Epoch = item.Epoch
 		existing.Text = item.Text
 		existing.Text = item.Text
-		if item.RegionId != 0 {
-			existing.RegionId = item.RegionId
+
+		if item.Epoch != 0 {
+			existing.Epoch = item.Epoch
+		}
+		if item.EpochEnd != 0 {
+			existing.EpochEnd = item.EpochEnd
+		}
+
+		if err := validateTimeRange(existing); err != nil {
+			return err
 		}
 		}
 
 
 		if item.Tags != nil {
 		if item.Tags != nil {
@@ -90,7 +114,7 @@ func (r *SqlAnnotationRepo) Update(item *annotations.Item) error {
 
 
 		existing.Tags = item.Tags
 		existing.Tags = item.Tags
 
 
-		_, err = sess.Table("annotation").ID(existing.Id).Cols("epoch", "text", "region_id", "updated", "tags").Update(existing)
+		_, err = sess.Table("annotation").ID(existing.Id).Cols("epoch", "text", "epoch_end", "updated", "tags").Update(existing)
 		return err
 		return err
 	})
 	})
 }
 }
@@ -103,12 +127,12 @@ func (r *SqlAnnotationRepo) Find(query *annotations.ItemQuery) ([]*annotations.I
 		SELECT
 		SELECT
 			annotation.id,
 			annotation.id,
 			annotation.epoch as time,
 			annotation.epoch as time,
+			annotation.epoch_end as time_end,
 			annotation.dashboard_id,
 			annotation.dashboard_id,
 			annotation.panel_id,
 			annotation.panel_id,
 			annotation.new_state,
 			annotation.new_state,
 			annotation.prev_state,
 			annotation.prev_state,
 			annotation.alert_id,
 			annotation.alert_id,
-			annotation.region_id,
 			annotation.text,
 			annotation.text,
 			annotation.tags,
 			annotation.tags,
 			annotation.data,
 			annotation.data,
@@ -131,11 +155,6 @@ func (r *SqlAnnotationRepo) Find(query *annotations.ItemQuery) ([]*annotations.I
 		params = append(params, query.AnnotationId)
 		params = append(params, query.AnnotationId)
 	}
 	}
 
 
-	if query.RegionId != 0 {
-		sql.WriteString(` AND annotation.region_id = ?`)
-		params = append(params, query.RegionId)
-	}
-
 	if query.AlertId != 0 {
 	if query.AlertId != 0 {
 		sql.WriteString(` AND annotation.alert_id = ?`)
 		sql.WriteString(` AND annotation.alert_id = ?`)
 		params = append(params, query.AlertId)
 		params = append(params, query.AlertId)
@@ -157,8 +176,8 @@ func (r *SqlAnnotationRepo) Find(query *annotations.ItemQuery) ([]*annotations.I
 	}
 	}
 
 
 	if query.From > 0 && query.To > 0 {
 	if query.From > 0 && query.To > 0 {
-		sql.WriteString(` AND annotation.epoch BETWEEN ? AND ?`)
-		params = append(params, query.From, query.To)
+		sql.WriteString(` AND annotation.epoch <= ? AND annotation.epoch_end >= ?`)
+		params = append(params, query.To, query.From)
 	}
 	}
 
 
 	if query.Type == "alert" {
 	if query.Type == "alert" {
@@ -224,11 +243,7 @@ func (r *SqlAnnotationRepo) Delete(params *annotations.DeleteParams) error {
 		)
 		)
 
 
 		sqlog.Info("delete", "orgId", params.OrgId)
 		sqlog.Info("delete", "orgId", params.OrgId)
-		if params.RegionId != 0 {
-			annoTagSql = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE region_id = ? AND org_id = ?)"
-			sql = "DELETE FROM annotation WHERE region_id = ? AND org_id = ?"
-			queryParams = []interface{}{params.RegionId, params.OrgId}
-		} else if params.Id != 0 {
+		if params.Id != 0 {
 			annoTagSql = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE id = ? AND org_id = ?)"
 			annoTagSql = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE id = ? AND org_id = ?)"
 			sql = "DELETE FROM annotation WHERE id = ? AND org_id = ?"
 			sql = "DELETE FROM annotation WHERE id = ? AND org_id = ?"
 			queryParams = []interface{}{params.Id, params.OrgId}
 			queryParams = []interface{}{params.Id, params.OrgId}

+ 5 - 13
pkg/services/sqlstore/annotation_test.go

@@ -35,6 +35,7 @@ func TestAnnotations(t *testing.T) {
 
 
 			So(err, ShouldBeNil)
 			So(err, ShouldBeNil)
 			So(annotation.Id, ShouldBeGreaterThan, 0)
 			So(annotation.Id, ShouldBeGreaterThan, 0)
+			So(annotation.Epoch, ShouldEqual, annotation.EpochEnd)
 
 
 			annotation2 := &annotations.Item{
 			annotation2 := &annotations.Item{
 				OrgId:       1,
 				OrgId:       1,
@@ -42,13 +43,15 @@ func TestAnnotations(t *testing.T) {
 				DashboardId: 2,
 				DashboardId: 2,
 				Text:        "hello",
 				Text:        "hello",
 				Type:        "alert",
 				Type:        "alert",
-				Epoch:       20,
+				Epoch:       21, // Should swap epoch & epochEnd
+				EpochEnd:    20,
 				Tags:        []string{"outage", "error", "type:outage", "server:server-1"},
 				Tags:        []string{"outage", "error", "type:outage", "server:server-1"},
-				RegionId:    1,
 			}
 			}
 			err = repo.Save(annotation2)
 			err = repo.Save(annotation2)
 			So(err, ShouldBeNil)
 			So(err, ShouldBeNil)
 			So(annotation2.Id, ShouldBeGreaterThan, 0)
 			So(annotation2.Id, ShouldBeGreaterThan, 0)
+			So(annotation2.Epoch, ShouldEqual, 20)
+			So(annotation2.EpochEnd, ShouldEqual, 21)
 
 
 			globalAnnotation1 := &annotations.Item{
 			globalAnnotation1 := &annotations.Item{
 				OrgId:  1,
 				OrgId:  1,
@@ -107,17 +110,6 @@ func TestAnnotations(t *testing.T) {
 				So(items[0].Id, ShouldEqual, annotation2.Id)
 				So(items[0].Id, ShouldEqual, annotation2.Id)
 			})
 			})
 
 
-			Convey("Can query for annotation by region id", func() {
-				items, err := repo.Find(&annotations.ItemQuery{
-					OrgId:    1,
-					RegionId: annotation2.RegionId,
-				})
-
-				So(err, ShouldBeNil)
-				So(items, ShouldHaveLength, 1)
-				So(items[0].Id, ShouldEqual, annotation2.Id)
-			})
-
 			Convey("Should not find any when item is outside time range", func() {
 			Convey("Should not find any when item is outside time range", func() {
 				items, err := repo.Find(&annotations.ItemQuery{
 				items, err := repo.Find(&annotations.ItemQuery{
 					OrgId:       1,
 					OrgId:       1,

+ 48 - 0
pkg/services/sqlstore/migrations/annotation_mig.go

@@ -1,6 +1,7 @@
 package migrations
 package migrations
 
 
 import (
 import (
+	"github.com/go-xorm/xorm"
 	. "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
 	. "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
 )
 )
 
 
@@ -109,4 +110,51 @@ func addAnnotationMig(mg *Migrator) {
 	//
 	//
 	updateEpochSql := "UPDATE annotation SET epoch = (epoch*1000) where epoch < 9999999999"
 	updateEpochSql := "UPDATE annotation SET epoch = (epoch*1000) where epoch < 9999999999"
 	mg.AddMigration("Convert existing annotations from seconds to milliseconds", NewRawSqlMigration(updateEpochSql))
 	mg.AddMigration("Convert existing annotations from seconds to milliseconds", NewRawSqlMigration(updateEpochSql))
+
+	//
+	// 6.4: Make Regions a single annotation row
+	//
+	mg.AddMigration("Add epoch_end column", NewAddColumnMigration(table, &Column{
+		Name: "epoch_end", Type: DB_BigInt, Nullable: false, Default: "0",
+	}))
+	mg.AddMigration("Add index for epoch_end", NewAddIndexMigration(table, &Index{
+		Cols: []string{"org_id", "epoch", "epoch_end"}, Type: IndexType,
+	}))
+	mg.AddMigration("Make epoch_end the same as epoch", NewRawSqlMigration("UPDATE annotation SET epoch_end = epoch"))
+	mg.AddMigration("Move region to single row", &AddMakeRegionSingleRowMigration{})
+
+	// TODO! drop region_id column?
+}
+
+type AddMakeRegionSingleRowMigration struct {
+	MigrationBase
+}
+
+func (m *AddMakeRegionSingleRowMigration) Sql(dialect Dialect) string {
+	return "code migration"
+}
+
+type TempRegionInfoDTO struct {
+	RegionId int64
+	Epoch    int64
+}
+
+func (m *AddMakeRegionSingleRowMigration) Exec(sess *xorm.Session, mg *Migrator) error {
+	regions := make([]*TempRegionInfoDTO, 0)
+
+	err := sess.SQL("SELECT region_id, epoch FROM annotation WHERE region_id>0 AND region_id <> id").Find(&regions)
+
+	if err != nil {
+		return err
+	}
+
+	for _, region := range regions {
+		_, err := sess.Exec("UPDATE annotation SET epoch_end = ? WHERE id = ?", region.Epoch, region.RegionId)
+		if err != nil {
+			return err
+		}
+	}
+
+	sess.Exec("DELETE FROM annotation WHERE region_id > 0 AND id <> region_id")
+	return nil
 }
 }

+ 8 - 8
public/app/features/annotations/annotations_srv.ts

@@ -7,14 +7,14 @@ import './editor_ctrl';
 import coreModule from 'app/core/core_module';
 import coreModule from 'app/core/core_module';
 
 
 // Utils & Services
 // Utils & Services
-import { makeRegions, dedupAnnotations } from './events_processing';
+import { dedupAnnotations } from './events_processing';
 
 
 // Types
 // Types
 import { DashboardModel } from '../dashboard/state/DashboardModel';
 import { DashboardModel } from '../dashboard/state/DashboardModel';
+import { AnnotationEvent } from '@grafana/data';
 import DatasourceSrv from '../plugins/datasource_srv';
 import DatasourceSrv from '../plugins/datasource_srv';
 import { BackendSrv } from 'app/core/services/backend_srv';
 import { BackendSrv } from 'app/core/services/backend_srv';
 import { TimeSrv } from '../dashboard/services/TimeSrv';
 import { TimeSrv } from '../dashboard/services/TimeSrv';
-import { AnnotationEvent } from '@grafana/data';
 
 
 export class AnnotationsSrv {
 export class AnnotationsSrv {
   globalAnnotationsPromise: any;
   globalAnnotationsPromise: any;
@@ -48,7 +48,7 @@ export class AnnotationsSrv {
       .all([this.getGlobalAnnotations(options), this.getAlertStates(options)])
       .all([this.getGlobalAnnotations(options), this.getAlertStates(options)])
       .then(results => {
       .then(results => {
         // combine the annotations and flatten results
         // combine the annotations and flatten results
-        let annotations: any[] = _.flattenDeep(results[0]);
+        let annotations: AnnotationEvent[] = _.flattenDeep(results[0]);
 
 
         // filter out annotations that do not belong to requesting panel
         // filter out annotations that do not belong to requesting panel
         annotations = _.filter(annotations, item => {
         annotations = _.filter(annotations, item => {
@@ -60,7 +60,10 @@ export class AnnotationsSrv {
         });
         });
 
 
         annotations = dedupAnnotations(annotations);
         annotations = dedupAnnotations(annotations);
-        annotations = makeRegions(annotations, options);
+        for (let i = 0; i < annotations.length; i++) {
+          const a = annotations[i];
+          a.isRegion = a.time !== a.timeEnd;
+        }
 
 
         // look for alert state for this panel
         // look for alert state for this panel
         const alertState: any = _.find(results[1], { panelId: options.panel.id });
         const alertState: any = _.find(results[1], { panelId: options.panel.id });
@@ -163,10 +166,7 @@ export class AnnotationsSrv {
 
 
   deleteAnnotationEvent(annotation: AnnotationEvent) {
   deleteAnnotationEvent(annotation: AnnotationEvent) {
     this.globalAnnotationsPromise = null;
     this.globalAnnotationsPromise = null;
-    let deleteUrl = `/api/annotations/${annotation.id}`;
-    if (annotation.isRegion) {
-      deleteUrl = `/api/annotations/region/${annotation.regionId}`;
-    }
+    const deleteUrl = `/api/annotations/${annotation.id}`;
 
 
     return this.backendSrv.delete(deleteUrl);
     return this.backendSrv.delete(deleteUrl);
   }
   }

+ 0 - 53
public/app/features/annotations/events_processing.ts

@@ -1,58 +1,5 @@
 import _ from 'lodash';
 import _ from 'lodash';
 
 
-/**
- * This function converts annotation events into set
- * of single events and regions (event consist of two)
- * @param annotations
- * @param options
- */
-export function makeRegions(annotations: any[], options: { range: any }) {
-  const [regionEvents, singleEvents] = _.partition(annotations, 'regionId');
-  const regions = getRegions(regionEvents, options.range);
-  annotations = _.concat(regions, singleEvents);
-  return annotations;
-}
-
-function getRegions(events: string | any[], range: { to: { valueOf: () => number }; from: { valueOf: () => number } }) {
-  const regionEvents = _.filter(events, event => {
-    return event.regionId;
-  });
-  let regions: any = _.groupBy(regionEvents, 'regionId');
-  regions = _.compact(
-    _.map(regions, regionEvents => {
-      const regionObj: any = _.head(regionEvents);
-      if (regionEvents && regionEvents.length > 1) {
-        regionObj.timeEnd = regionEvents[1].time;
-        regionObj.isRegion = true;
-        return regionObj;
-      } else {
-        if (regionEvents && regionEvents.length) {
-          // Don't change proper region object
-          if (!regionObj.time || !regionObj.timeEnd) {
-            // This is cut region
-            if (isStartOfRegion(regionObj)) {
-              regionObj.timeEnd = range.to.valueOf() - 1;
-            } else {
-              // Start time = null
-              regionObj.timeEnd = regionObj.time;
-              regionObj.time = range.from.valueOf() + 1;
-            }
-            regionObj.isRegion = true;
-          }
-
-          return regionObj;
-        }
-      }
-    })
-  );
-
-  return regions;
-}
-
-function isStartOfRegion(event: { id: any; regionId: any }): boolean {
-  return event.id && event.id === event.regionId;
-}
-
 export function dedupAnnotations(annotations: any) {
 export function dedupAnnotations(annotations: any) {
   let dedup = [];
   let dedup = [];
 
 

+ 27 - 66
public/app/features/annotations/specs/annotations_srv_specs.test.ts

@@ -1,70 +1,31 @@
-import { makeRegions, dedupAnnotations } from '../events_processing';
-
-describe('Annotations', () => {
-  describe('Annotations regions', () => {
-    let testAnnotations: any[];
-
-    beforeEach(() => {
-      testAnnotations = [
-        { id: 1, time: 1 },
-        { id: 2, time: 2 },
-        { id: 3, time: 3, regionId: 3 },
-        { id: 4, time: 5, regionId: 3 },
-        { id: 5, time: 4, regionId: 5 },
-        { id: 6, time: 8, regionId: 5 },
-      ];
-    });
-
-    it('should convert single region events to regions', () => {
-      const range = { from: 0, to: 10 };
-      const expectedAnnotations = [
-        { id: 3, regionId: 3, isRegion: true, time: 3, timeEnd: 5 },
-        { id: 5, regionId: 5, isRegion: true, time: 4, timeEnd: 8 },
-        { id: 1, time: 1 },
-        { id: 2, time: 2 },
-      ];
-
-      const regions = makeRegions(testAnnotations, { range: range });
-      expect(regions).toEqual(expectedAnnotations);
-    });
-
-    it('should cut regions to current time range', () => {
-      const range = { from: 0, to: 8 };
-      testAnnotations = [{ id: 5, time: 4, regionId: 5 }];
-      const expectedAnnotations = [{ id: 5, regionId: 5, isRegion: true, time: 4, timeEnd: 7 }];
-
-      const regions = makeRegions(testAnnotations, { range: range });
-      expect(regions).toEqual(expectedAnnotations);
-    });
+import { dedupAnnotations } from '../events_processing';
+
+describe('Annotations deduplication', () => {
+  it('should remove duplicated annotations', () => {
+    const testAnnotations = [
+      { id: 1, time: 1 },
+      { id: 2, time: 2 },
+      { id: 2, time: 2 },
+      { id: 5, time: 5 },
+      { id: 5, time: 5 },
+    ];
+    const expectedAnnotations = [{ id: 1, time: 1 }, { id: 2, time: 2 }, { id: 5, time: 5 }];
+
+    const deduplicated = dedupAnnotations(testAnnotations);
+    expect(deduplicated).toEqual(expectedAnnotations);
   });
   });
 
 
-  describe('Annotations deduplication', () => {
-    it('should remove duplicated annotations', () => {
-      const testAnnotations = [
-        { id: 1, time: 1 },
-        { id: 2, time: 2 },
-        { id: 2, time: 2 },
-        { id: 5, time: 5 },
-        { id: 5, time: 5 },
-      ];
-      const expectedAnnotations = [{ id: 1, time: 1 }, { id: 2, time: 2 }, { id: 5, time: 5 }];
-
-      const deduplicated = dedupAnnotations(testAnnotations);
-      expect(deduplicated).toEqual(expectedAnnotations);
-    });
-
-    it('should leave non "panel-alert" event if present', () => {
-      const testAnnotations = [
-        { id: 1, time: 1 },
-        { id: 2, time: 2 },
-        { id: 2, time: 2, eventType: 'panel-alert' },
-        { id: 5, time: 5 },
-        { id: 5, time: 5 },
-      ];
-      const expectedAnnotations = [{ id: 1, time: 1 }, { id: 2, time: 2 }, { id: 5, time: 5 }];
-
-      const deduplicated = dedupAnnotations(testAnnotations);
-      expect(deduplicated).toEqual(expectedAnnotations);
-    });
+  it('should leave non "panel-alert" event if present', () => {
+    const testAnnotations = [
+      { id: 1, time: 1 },
+      { id: 2, time: 2 },
+      { id: 2, time: 2, eventType: 'panel-alert' },
+      { id: 5, time: 5 },
+      { id: 5, time: 5 },
+    ];
+    const expectedAnnotations = [{ id: 1, time: 1 }, { id: 2, time: 2 }, { id: 5, time: 5 }];
+
+    const deduplicated = dedupAnnotations(testAnnotations);
+    expect(deduplicated).toEqual(expectedAnnotations);
   });
   });
 });
 });