From 3a607f96a3b102aacd4e24ba334442c099890cce Mon Sep 17 00:00:00 2001 From: Dan Cech Date: Fri, 14 Apr 2017 01:45:36 -0400 Subject: [PATCH 1/5] fix bug in log sprintf calls (#8124) --- pkg/log/log.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/log/log.go b/pkg/log/log.go index fe0b312db23..f69d0f6b9b4 100644 --- a/pkg/log/log.go +++ b/pkg/log/log.go @@ -34,7 +34,7 @@ func New(logger string, ctx ...interface{}) Logger { func Trace(format string, v ...interface{}) { var message string if len(v) > 0 { - message = fmt.Sprintf(format, v) + message = fmt.Sprintf(format, v...) } else { message = format } @@ -45,7 +45,7 @@ func Trace(format string, v ...interface{}) { func Debug(format string, v ...interface{}) { var message string if len(v) > 0 { - message = fmt.Sprintf(format, v) + message = fmt.Sprintf(format, v...) } else { message = format } @@ -60,7 +60,7 @@ func Debug2(message string, v ...interface{}) { func Info(format string, v ...interface{}) { var message string if len(v) > 0 { - message = fmt.Sprintf(format, v) + message = fmt.Sprintf(format, v...) } else { message = format } @@ -75,7 +75,7 @@ func Info2(message string, v ...interface{}) { func Warn(format string, v ...interface{}) { var message string if len(v) > 0 { - message = fmt.Sprintf(format, v) + message = fmt.Sprintf(format, v...) } else { message = format } @@ -88,7 +88,7 @@ func Warn2(message string, v ...interface{}) { } func Error(skip int, format string, v ...interface{}) { - Root.Error(fmt.Sprintf(format, v)) + Root.Error(fmt.Sprintf(format, v...)) } func Error2(message string, v ...interface{}) { @@ -96,7 +96,7 @@ func Error2(message string, v ...interface{}) { } func Critical(skip int, format string, v ...interface{}) { - Root.Crit(fmt.Sprintf(format, v)) + Root.Crit(fmt.Sprintf(format, v...)) } func Fatal(skip int, format string, v ...interface{}) { From fb163450a5a980cc3344edd103d55ae9620f7953 Mon Sep 17 00:00:00 2001 From: Ivan Babrou Date: Fri, 14 Apr 2017 05:51:22 -0700 Subject: [PATCH 2/5] Change prometheus semantics from step to min step (#8073) Previously `Step` parameter would set a hard value for any zoom level. Now it's renamed to `Min step` and sets the minimal value of `step` parameter to Prometheus query. User would usually want to set it to the scraping interval of the target metric to avoid having shap cliffs on graphs and extra load on Prometheus. Actual `step` value is calculated as the minimum of automatically selected step (based on zoom level) and user provided minimal step. If user did not provide the step, then automatic value is used as is. Example bahavior for `60s` scrape intervals: * `5s` automatic interval, no user specified min step: * Before: `step=5` * After: `step=5` * `5s` automatic interval, `1m` user specified min step: * Before: `step=5` * After: `step=60` * `5m` automatic interval, `1m` user specified min step: * Before: `step=60` (not really visible, too dense) * After: `step=300` (automatic value is picked) See: * https://github.com/grafana/grafana/issues/8065 * https://github.com/prometheus/prometheus/issues/2564 --- CHANGELOG.md | 1 + .../plugins/datasource/prometheus/datasource.ts | 14 +++++++++----- .../prometheus/partials/query.editor.html | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c32dcdba40..128abbb1662 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ## Minor Enchancements * **Prometheus**: Make Prometheus query field a textarea [#7663](https://github.com/grafana/grafana/issues/7663), thx [@hagen1778](https://github.com/hagen1778) +* **Prometheus**: Step parameter changed semantics to min step to reduce the load on Prometheus and rendering in browser [#8073](https://github.com/grafana/grafana/pull/8073), thx [@bobrik](https://github.com/bobrik) * **Templating**: Should not be possible to create self-referencing (recursive) template variable definitions [#7614](https://github.com/grafana/grafana/issues/7614) thx [@thuck](https://github.com/thuck) * **Cloudwatch**: Correctly obtain IAM roles within ECS container tasks [#7892](https://github.com/grafana/grafana/issues/7892) thx [@gomlgs](https://github.com/gomlgs) * **Units**: New number format: Scientific notation [#7781](https://github.com/grafana/grafana/issues/7781) thx [@cadnce](https://github.com/cadnce) diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index f883b40e5ee..3cd065719c7 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -89,7 +89,7 @@ export function PrometheusDatasource(instanceSettings, $q, backendSrv, templateS var intervalFactor = target.intervalFactor || 1; target.step = query.step = this.calculateInterval(interval, intervalFactor); var range = Math.ceil(end - start); - target.step = query.step = this.adjustStep(query.step, range); + target.step = query.step = this.adjustStep(query.step, this.intervalSeconds(options.interval), range); queries.push(query); }); @@ -122,13 +122,13 @@ export function PrometheusDatasource(instanceSettings, $q, backendSrv, templateS }); }; - this.adjustStep = function(step, range) { + this.adjustStep = function(step, autoStep, range) { // Prometheus drop query if range/step > 11000 // calibrate step if it is too big if (step !== 0 && range / step > 11000) { return Math.ceil(range / 11000); } - return step; + return Math.max(step, autoStep); }; this.performTimeSeriesQuery = function(query, start, end) { @@ -189,7 +189,7 @@ export function PrometheusDatasource(instanceSettings, $q, backendSrv, templateS var end = this.getPrometheusTime(options.range.to, true); var query = { expr: interpolated, - step: this.adjustStep(kbn.interval_to_seconds(step), Math.ceil(end - start)) + 's' + step: this.adjustStep(kbn.interval_to_seconds(step), 0, Math.ceil(end - start)) + 's' }; var self = this; @@ -229,6 +229,10 @@ export function PrometheusDatasource(instanceSettings, $q, backendSrv, templateS }; this.calculateInterval = function(interval, intervalFactor) { + return Math.ceil(this.intervalSeconds(interval) * intervalFactor); + }; + + this.intervalSeconds = function(interval) { var m = interval.match(durationSplitRegexp); var dur = moment.duration(parseInt(m[1]), m[2]); var sec = dur.asSeconds(); @@ -236,7 +240,7 @@ export function PrometheusDatasource(instanceSettings, $q, backendSrv, templateS sec = 1; } - return Math.ceil(sec * intervalFactor); + return sec; }; this.transformMetricData = function(md, options, start, end) { diff --git a/public/app/plugins/datasource/prometheus/partials/query.editor.html b/public/app/plugins/datasource/prometheus/partials/query.editor.html index 1840f74bd74..98bb359a093 100644 --- a/public/app/plugins/datasource/prometheus/partials/query.editor.html +++ b/public/app/plugins/datasource/prometheus/partials/query.editor.html @@ -14,7 +14,7 @@
- + Date: Fri, 14 Apr 2017 09:47:39 -0400 Subject: [PATCH 3/5] use X-Grafana-Org-Id header to ensure backend uses correct org (#8122) --- pkg/middleware/auth_proxy.go | 5 +- pkg/middleware/middleware.go | 77 ++++++++++++++----------- pkg/models/user.go | 1 + pkg/services/sqlstore/user.go | 32 +++++----- public/app/core/services/backend_srv.ts | 14 ++++- 5 files changed, 77 insertions(+), 52 deletions(-) diff --git a/pkg/middleware/auth_proxy.go b/pkg/middleware/auth_proxy.go index e02e31f9152..8e94e1582b0 100644 --- a/pkg/middleware/auth_proxy.go +++ b/pkg/middleware/auth_proxy.go @@ -13,7 +13,7 @@ import ( "github.com/grafana/grafana/pkg/setting" ) -func initContextWithAuthProxy(ctx *Context) bool { +func initContextWithAuthProxy(ctx *Context, orgId int64) bool { if !setting.AuthProxyEnabled { return false } @@ -30,6 +30,7 @@ func initContextWithAuthProxy(ctx *Context) bool { } query := getSignedInUserQueryForProxyAuth(proxyHeaderValue) + query.OrgId = orgId if err := bus.Dispatch(query); err != nil { if err != m.ErrUserNotFound { ctx.Handle(500, "Failed to find user specified in auth proxy header", err) @@ -46,7 +47,7 @@ func initContextWithAuthProxy(ctx *Context) bool { ctx.Handle(500, "Failed to create user specified in auth proxy header", err) return true } - query = &m.GetSignedInUserQuery{UserId: cmd.Result.Id} + query = &m.GetSignedInUserQuery{UserId: cmd.Result.Id, OrgId: orgId} if err := bus.Dispatch(query); err != nil { ctx.Handle(500, "Failed find user after creation", err) return true diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 4b59fada62e..5aafe12d374 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -39,6 +39,12 @@ func GetContextHandler() macaron.Handler { Logger: log.New("context"), } + orgId := int64(0) + orgIdHeader := ctx.Req.Header.Get("X-Grafana-Org-Id") + if orgIdHeader != "" { + orgId, _ = strconv.ParseInt(orgIdHeader, 10, 64) + } + // the order in which these are tested are important // look for api key in Authorization header first // then init session and look for userId in session @@ -46,9 +52,9 @@ func GetContextHandler() macaron.Handler { // then test if anonymous access is enabled if initContextWithRenderAuth(ctx) || initContextWithApiKey(ctx) || - initContextWithBasicAuth(ctx) || - initContextWithAuthProxy(ctx) || - initContextWithUserSessionCookie(ctx) || + initContextWithBasicAuth(ctx, orgId) || + initContextWithAuthProxy(ctx, orgId) || + initContextWithUserSessionCookie(ctx, orgId) || initContextWithAnonymousUser(ctx) { } @@ -68,18 +74,18 @@ func initContextWithAnonymousUser(ctx *Context) bool { if err := bus.Dispatch(&orgQuery); err != nil { log.Error(3, "Anonymous access organization error: '%s': %s", setting.AnonymousOrgName, err) return false - } else { - ctx.IsSignedIn = false - ctx.AllowAnonymous = true - ctx.SignedInUser = &m.SignedInUser{} - ctx.OrgRole = m.RoleType(setting.AnonymousOrgRole) - ctx.OrgId = orgQuery.Result.Id - ctx.OrgName = orgQuery.Result.Name - return true } + + ctx.IsSignedIn = false + ctx.AllowAnonymous = true + ctx.SignedInUser = &m.SignedInUser{} + ctx.OrgRole = m.RoleType(setting.AnonymousOrgRole) + ctx.OrgId = orgQuery.Result.Id + ctx.OrgName = orgQuery.Result.Name + return true } -func initContextWithUserSessionCookie(ctx *Context) bool { +func initContextWithUserSessionCookie(ctx *Context, orgId int64) bool { // initialize session if err := ctx.Session.Start(ctx); err != nil { ctx.Logger.Error("Failed to start session", "error", err) @@ -91,15 +97,15 @@ func initContextWithUserSessionCookie(ctx *Context) bool { return false } - query := m.GetSignedInUserQuery{UserId: userId} + query := m.GetSignedInUserQuery{UserId: userId, OrgId: orgId} if err := bus.Dispatch(&query); err != nil { ctx.Logger.Error("Failed to get user with id", "userId", userId) return false - } else { - ctx.SignedInUser = query.Result - ctx.IsSignedIn = true - return true } + + ctx.SignedInUser = query.Result + ctx.IsSignedIn = true + return true } func initContextWithApiKey(ctx *Context) bool { @@ -114,30 +120,31 @@ func initContextWithApiKey(ctx *Context) bool { ctx.JsonApiErr(401, "Invalid API key", err) return true } + // fetch key keyQuery := m.GetApiKeyByNameQuery{KeyName: decoded.Name, OrgId: decoded.OrgId} if err := bus.Dispatch(&keyQuery); err != nil { ctx.JsonApiErr(401, "Invalid API key", err) return true - } else { - apikey := keyQuery.Result + } - // validate api key - if !apikeygen.IsValid(decoded, apikey.Key) { - ctx.JsonApiErr(401, "Invalid API key", err) - return true - } + apikey := keyQuery.Result - ctx.IsSignedIn = true - ctx.SignedInUser = &m.SignedInUser{} - ctx.OrgRole = apikey.Role - ctx.ApiKeyId = apikey.Id - ctx.OrgId = apikey.OrgId + // validate api key + if !apikeygen.IsValid(decoded, apikey.Key) { + ctx.JsonApiErr(401, "Invalid API key", err) return true } + + ctx.IsSignedIn = true + ctx.SignedInUser = &m.SignedInUser{} + ctx.OrgRole = apikey.Role + ctx.ApiKeyId = apikey.Id + ctx.OrgId = apikey.OrgId + return true } -func initContextWithBasicAuth(ctx *Context) bool { +func initContextWithBasicAuth(ctx *Context, orgId int64) bool { if !setting.BasicAuthEnabled { return false @@ -168,15 +175,15 @@ func initContextWithBasicAuth(ctx *Context) bool { return true } - query := m.GetSignedInUserQuery{UserId: user.Id} + query := m.GetSignedInUserQuery{UserId: user.Id, OrgId: orgId} if err := bus.Dispatch(&query); err != nil { ctx.JsonApiErr(401, "Authentication error", err) return true - } else { - ctx.SignedInUser = query.Result - ctx.IsSignedIn = true - return true } + + ctx.SignedInUser = query.Result + ctx.IsSignedIn = true + return true } // Handle handles and logs error by given status. diff --git a/pkg/models/user.go b/pkg/models/user.go index e0a36be8c0a..bdf81056232 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -117,6 +117,7 @@ type GetSignedInUserQuery struct { UserId int64 Login string Email string + OrgId int64 Result *SignedInUser } diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 9a44d6f194e..71b29bd3355 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -1,6 +1,7 @@ package sqlstore import ( + "strconv" "strings" "time" @@ -273,7 +274,7 @@ func SetUsingOrg(cmd *m.SetUsingOrgCommand) error { } if !valid { - return fmt.Errorf("user does not belong ot org") + return fmt.Errorf("user does not belong to org") } return inTransaction(func(sess *xorm.Session) error { @@ -319,19 +320,24 @@ func GetUserOrgList(query *m.GetUserOrgListQuery) error { } func GetSignedInUser(query *m.GetSignedInUserQuery) error { + orgId := "u.org_id" + if query.OrgId > 0 { + orgId = strconv.FormatInt(query.OrgId, 10) + } + var rawSql = `SELECT - u.id as user_id, - u.is_admin as is_grafana_admin, - u.email as email, - u.login as login, - u.name as name, - u.help_flags1 as help_flags1, - org.name as org_name, - org_user.role as org_role, - org.id as org_id - FROM ` + dialect.Quote("user") + ` as u - LEFT OUTER JOIN org_user on org_user.org_id = u.org_id and org_user.user_id = u.id - LEFT OUTER JOIN org on org.id = u.org_id ` + u.id as user_id, + u.is_admin as is_grafana_admin, + u.email as email, + u.login as login, + u.name as name, + u.help_flags1 as help_flags1, + org.name as org_name, + org_user.role as org_role, + org.id as org_id + FROM ` + dialect.Quote("user") + ` as u + LEFT OUTER JOIN org_user on org_user.org_id = ` + orgId + ` and org_user.user_id = u.id + LEFT OUTER JOIN org on org.id = org_user.org_id ` sess := x.Table("user") if query.UserId > 0 { diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index ba4f4dc2fb9..cc52de4147c 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -9,8 +9,8 @@ export class BackendSrv { inFlightRequests = {}; HTTP_REQUEST_CANCELLED = -1; - /** @ngInject */ - constructor(private $http, private alertSrv, private $rootScope, private $q, private $timeout) { + /** @ngInject */ + constructor(private $http, private alertSrv, private $rootScope, private $q, private $timeout, private contextSrv) { } get(url, params?) { @@ -66,6 +66,11 @@ export class BackendSrv { var requestIsLocal = options.url.indexOf('/') === 0; var firstAttempt = options.retry === 0; + if (!options.url.match('https?://') && this.contextSrv && this.contextSrv.user && this.contextSrv.user.orgId) { + options.headers = options.headers || {}; + options.headers['X-Grafana-Org-Id'] = this.contextSrv.user.orgId; + } + if (requestIsLocal && !options.hasSubUrl) { options.url = config.appSubUrl + options.url; options.hasSubUrl = true; @@ -128,6 +133,11 @@ export class BackendSrv { var requestIsLocal = options.url.indexOf('/') === 0; var firstAttempt = options.retry === 0; + if (!options.url.match('https?://') && this.contextSrv && this.contextSrv.user && this.contextSrv.user.orgId) { + options.headers = options.headers || {}; + options.headers['X-Grafana-Org-Id'] = this.contextSrv.user.orgId; + } + if (requestIsLocal && !options.hasSubUrl && options.retry === 0) { options.url = config.appSubUrl + options.url; } From b77991f69b22f590518f41f5a0f44e80155ab0c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Fri, 14 Apr 2017 16:00:52 +0200 Subject: [PATCH 4/5] refactoring: minor changes to #8122 --- public/app/core/services/backend_srv.ts | 43 ++++++++++++++----------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index cc52de4147c..3159e87882b 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -63,17 +63,20 @@ export class BackendSrv { request(options) { options.retry = options.retry || 0; - var requestIsLocal = options.url.indexOf('/') === 0; + var requestIsLocal = !options.url.match(/^http/); var firstAttempt = options.retry === 0; - if (!options.url.match('https?://') && this.contextSrv && this.contextSrv.user && this.contextSrv.user.orgId) { - options.headers = options.headers || {}; - options.headers['X-Grafana-Org-Id'] = this.contextSrv.user.orgId; - } - if (requestIsLocal && !options.hasSubUrl) { - options.url = config.appSubUrl + options.url; - options.hasSubUrl = true; + if (requestIsLocal) { + if (this.contextSrv.user && this.contextSrv.user.orgId) { + options.headers = options.headers || {}; + options.headers['X-Grafana-Org-Id'] = this.contextSrv.user.orgId; + } + + if (!options.hasSubUrl) { + options.url = config.appSubUrl + options.url; + options.hasSubUrl = true; + } } return this.$http(options).then(results => { @@ -130,21 +133,23 @@ export class BackendSrv { this.addCanceler(requestId, canceler); } - var requestIsLocal = options.url.indexOf('/') === 0; + var requestIsLocal = !options.url.match(/^http/); var firstAttempt = options.retry === 0; - if (!options.url.match('https?://') && this.contextSrv && this.contextSrv.user && this.contextSrv.user.orgId) { - options.headers = options.headers || {}; - options.headers['X-Grafana-Org-Id'] = this.contextSrv.user.orgId; - } + if (requestIsLocal) { + if (this.contextSrv.user && this.contextSrv.user.orgId) { + options.headers = options.headers || {}; + options.headers['X-Grafana-Org-Id'] = this.contextSrv.user.orgId; + } - if (requestIsLocal && !options.hasSubUrl && options.retry === 0) { - options.url = config.appSubUrl + options.url; - } + if (!options.hasSubUrl && options.retry === 0) { + options.url = config.appSubUrl + options.url; + } - if (requestIsLocal && options.headers && options.headers.Authorization) { - options.headers['X-DS-Authorization'] = options.headers.Authorization; - delete options.headers.Authorization; + if (options.headers && options.headers.Authorization) { + options.headers['X-DS-Authorization'] = options.headers.Authorization; + delete options.headers.Authorization; + } } return this.$http(options).catch(err => { From aa47b9bf5c02a9c850277ec3d25fcd81c721b736 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Fri, 14 Apr 2017 19:01:08 +0200 Subject: [PATCH 5/5] refactoring: simplified backend_srv and subUrl handling, #8122 --- public/app/core/services/backend_srv.ts | 10 ++++------ public/app/plugins/panel/gettingstarted/module.ts | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index 3159e87882b..041cd1ab1db 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -66,16 +66,14 @@ export class BackendSrv { var requestIsLocal = !options.url.match(/^http/); var firstAttempt = options.retry === 0; - if (requestIsLocal) { if (this.contextSrv.user && this.contextSrv.user.orgId) { options.headers = options.headers || {}; options.headers['X-Grafana-Org-Id'] = this.contextSrv.user.orgId; } - if (!options.hasSubUrl) { - options.url = config.appSubUrl + options.url; - options.hasSubUrl = true; + if (options.url.indexOf("/") === 0) { + options.url = options.url.substring(1); } } @@ -142,8 +140,8 @@ export class BackendSrv { options.headers['X-Grafana-Org-Id'] = this.contextSrv.user.orgId; } - if (!options.hasSubUrl && options.retry === 0) { - options.url = config.appSubUrl + options.url; + if (options.url.indexOf("/") === 0) { + options.url = options.url.substring(1); } if (options.headers && options.headers.Authorization) { diff --git a/public/app/plugins/panel/gettingstarted/module.ts b/public/app/plugins/panel/gettingstarted/module.ts index 4ebc76f942e..f0ee0bf36e4 100644 --- a/public/app/plugins/panel/gettingstarted/module.ts +++ b/public/app/plugins/panel/gettingstarted/module.ts @@ -58,7 +58,7 @@ class GettingStartedPanelCtrl extends PanelCtrl { icon: 'icon-gf icon-gf-users', href: 'org/users?gettingstarted', check: () => { - return this.backendSrv.get('api/org/users').then(res => { + return this.backendSrv.get('/api/org/users').then(res => { return res.length > 1; }); } @@ -71,7 +71,7 @@ class GettingStartedPanelCtrl extends PanelCtrl { icon: 'icon-gf icon-gf-apps', href: 'https://grafana.com/plugins?utm_source=grafana_getting_started', check: () => { - return this.backendSrv.get('api/plugins', {embedded: 0, core: 0}).then(plugins => { + return this.backendSrv.get('/api/plugins', {embedded: 0, core: 0}).then(plugins => { return plugins.length > 0; }); }