Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

tags/findSeries - add lastts-json format #1580

Merged
merged 11 commits into from
Jan 14, 2020
Merged

tags/findSeries - add lastts-json format #1580

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0faa5a5
tags/findSeries - add lastupdate-json format
shanson7 Dec 23, 2019
c1b4cf9
Add limit param to maintain default behavior
shanson7 Dec 24, 2019
d24b2bc
Update HTTP docs
shanson7 Dec 23, 2019
9c8fdbe
Respect disabled max-series-per-req
shanson7 Dec 27, 2019
a198675
Rename lastupdate to lastts in format
shanson7 Jan 6, 2020
a0c7766
Don't error for soft limit
shanson7 Jan 6, 2020
8c685e0
Don't generate msgp for unneeded types
shanson7 Jan 6, 2020
7c2bca7
Interface tweaks
shanson7 Jan 8, 2020
8064f48
Update http api docs with meta
shanson7 Jan 8, 2020
cddafc4
wording
Dieterbe Jan 9, 2020
1a134a2
Clean up the limiting logic
shanson7 Jan 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 60 additions & 13 deletions api/graphite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ var MissingOrgHeaderErr = errors.New("orgId not set in headers")
var MissingQueryErr = errors.New("missing query param")
var InvalidFormatErr = errors.New("invalid format specified")
var InvalidTimeRangeErr = errors.New("invalid time range requested")
var TooManySeriesErr = response.NewError(
http.StatusRequestEntityTooLarge,
Dieterbe marked this conversation as resolved.
Show resolved Hide resolved
"Request exceeds max-series-per-req limit. Reduce the number of targets or ask your admin to increase the limit.")

var renderReqProxied = stats.NewCounter32("api.request.render.proxied")

var (
Expand Down Expand Up @@ -687,7 +691,12 @@ func (s *Server) executePlan(ctx context.Context, orgId uint32, plan expr.Plan)
return nil, meta, err
}

series, err = s.clusterFindByTag(ctx, orgId, exprs, int64(r.From), maxSeriesPerReq-len(reqs))
remainingSeriesLimit := maxSeriesPerReq - len(reqs)
if remainingSeriesLimit <= 0 && maxSeriesPerReq > 0 {
// Use 1 to enable series count checking.
Dieterbe marked this conversation as resolved.
Show resolved Hide resolved
remainingSeriesLimit = 1
}
series, err = s.clusterFindByTag(ctx, orgId, exprs, int64(r.From), remainingSeriesLimit, false)
} else {
series, err = s.findSeries(ctx, orgId, []string{r.Query}, int64(r.From))
}
Expand Down Expand Up @@ -1012,8 +1021,18 @@ func (s *Server) graphiteTagFindSeries(ctx *middleware.Context, request models.G
return
}

series, err := s.clusterFindByTag(reqCtx, ctx.OrgId, expressions, request.From, maxSeriesPerReq)
if err != nil {
// If limit is specified and less than the global `maxSeriesPerReq` (or `maxSeriesPerReq` is disabled),
// then this is a "soft" limit, meaning we stop and don't return an error. If global `maxSeriesPerReq`
// exists, then respect that
isSoftLimit := true
limit := request.Limit
if maxSeriesPerReq > 0 && (limit == 0 || limit > maxSeriesPerReq) {
limit = maxSeriesPerReq
isSoftLimit = false
}

series, err := s.clusterFindByTag(reqCtx, ctx.OrgId, expressions, request.From, limit, isSoftLimit)
if err != nil && (!isSoftLimit || err != TooManySeriesErr) {
response.Write(ctx, response.WrapError(err))
return
}
Expand All @@ -1025,14 +1044,34 @@ func (s *Server) graphiteTagFindSeries(ctx *middleware.Context, request models.G
return
default:
}
seriesNames := make([]string, 0, len(series))
for _, serie := range series {
seriesNames = append(seriesNames, serie.Pattern)

switch request.Format {
case "lastupdate-json":
seriesVals := make([]models.SeriesLastUpdate, 0, len(series))
for _, serie := range series {
var lastUpdate int64
for _, node := range serie.Series {
for _, ndef := range node.Defs {
if ndef.LastUpdate > lastUpdate {
lastUpdate = ndef.LastUpdate
}
}
}
seriesVals = append(seriesVals, models.SeriesLastUpdate{Series: serie.Pattern, Ts: lastUpdate})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm a bit confused here. doesn't this only include the series name (without tags) ? I thought you wanted it broken down by tags.
also, it looks like this doesn't include meta tags.

Copy link
Collaborator Author

@shanson7 shanson7 Jan 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns the same string that is currently returned (i.e. the series-json format), which is formatted like name;tag1=val1;tag2=val2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@replay shouldn't we expand the name here to include meta tags?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of a good reason why it would be necessary to include meta tags here, except for consistency so that whoever uses the endpoint also gets to see the meta tags.
Note that in order to include meta tags in this result the index/find_by_tags endpoint and the MemoryIdx.FindByTag() method would all have to be modified to include meta tags in the returned results, which would slow finds down because those results would need to be enriched. So if we want to enable the user to also retrieve the meta tags on this endpoint we should make this an optional feature of the index/find_by_tags endpoint which by default is off.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I would say including meta tags here would be a separate feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I just looked at the related code again, and the series returned by clusterFindByTag already have the meta tags if the feature is enabled. They're in the .MetaTags property. So if that's wanted, it would be easy to also add them.

}
response.Write(ctx, response.NewJson(200, models.GraphiteTagFindSeriesLastUpdateResp{Series: seriesVals}, ""))
default:
// Backwards compatibility note:
// Before `Format` was added, it was ignored if specified. So, treat unknown formats as `seriesjson`.
seriesNames := make([]string, 0, len(series))
for _, serie := range series {
seriesNames = append(seriesNames, serie.Pattern)
}
response.Write(ctx, response.NewJson(200, seriesNames, ""))
}
response.Write(ctx, response.NewJson(200, seriesNames, ""))
}

func (s *Server) clusterFindByTag(ctx context.Context, orgId uint32, expressions tagquery.Expressions, from int64, maxSeries int) ([]Series, error) {
func (s *Server) clusterFindByTag(ctx context.Context, orgId uint32, expressions tagquery.Expressions, from int64, maxSeries int, softLimit bool) ([]Series, error) {
data := models.IndexFindByTag{OrgId: orgId, Expr: expressions.Strings(), From: from}
newCtx, cancel := context.WithCancel(ctx)
defer cancel()
Expand All @@ -1048,11 +1087,19 @@ func (s *Server) clusterFindByTag(ctx context.Context, orgId uint32, expressions
}

// 0 disables the check, so only check if maxSeriesPerReq > 0
if maxSeriesPerReq > 0 && len(resp.Metrics)+len(allSeries) > maxSeries {
return nil,
response.NewError(
http.StatusRequestEntityTooLarge,
fmt.Sprintf("Request exceeds max-series-per-req limit (%d). Reduce the number of targets or ask your admin to increase the limit.", maxSeriesPerReq))
if maxSeries > 0 && len(resp.Metrics)+len(allSeries) > maxSeries {
if softLimit {
remainingSpace := maxSeries - len(allSeries)
// Fill in up to maxSeries
for _, series := range resp.Metrics[:remainingSpace] {
allSeries = append(allSeries, Series{
Pattern: series.Path,
Node: r.peer,
Series: []idx.Node{series},
})
}
}
return allSeries, TooManySeriesErr
Dieterbe marked this conversation as resolved.
Show resolved Hide resolved
}

for _, series := range resp.Metrics {
Expand Down
15 changes: 13 additions & 2 deletions api/models/graphite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,25 @@ type GraphiteTagDetailsValueResp struct {
}

type GraphiteTagFindSeries struct {
Expr []string `json:"expr" form:"expr"`
From int64 `json:"from" form:"from"`
Expr []string `json:"expr" form:"expr"`
From int64 `json:"from" form:"from"`
Format string `json:"format" form:"format" binding:"Default(seriesjson)"`
Dieterbe marked this conversation as resolved.
Show resolved Hide resolved
Limit int `json:"limit" binding:"Default(0)"`
}

type GraphiteTagFindSeriesResp struct {
Series []string `json:"series"`
}

type SeriesLastUpdate struct {
Series string `json:"val"`
Ts int64 `json:"lastTs"`
}

type GraphiteTagFindSeriesLastUpdateResp struct {
Series []SeriesLastUpdate `json:"series"`
}

type GraphiteTagDelSeries struct {
Paths []string `json:"path" form:"path"`
Propagate bool `json:"propagate" form:"propagate" binding:"Default(true)"`
Expand Down
Loading