Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove prometheus labels with high cardinality #2701

Merged
merged 1 commit into from
Jun 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 3 additions & 10 deletions internal/ingress/metric/collector/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,10 @@ type socketData struct {
Host string `json:"host"` // Label
Status string `json:"status"` // Label

RealIPAddress string `json:"realIpAddr"` // Label
RemoteAddress string `json:"remoteAddr"` // Label
RemoteUser string `json:"remoteUser"` // Label

BytesSent float64 `json:"bytesSent"` // Metric

Protocol string `json:"protocol"` // Label
Method string `json:"method"` // Label
URI string `json:"uri"` // Label

RequestLength float64 `json:"requestLength"` // Metric
RequestTime float64 `json:"requestTime"` // Metric
Expand All @@ -51,6 +46,7 @@ type socketData struct {
Namespace string `json:"namespace"` // Label
Ingress string `json:"ingress"` // Label
Service string `json:"service"` // Label
Path string `json:"path"` // Label
}

// SocketCollector stores prometheus metrics and ingress meta-data
Expand Down Expand Up @@ -82,7 +78,7 @@ func NewInstance(ns string, class string) error {
sc.ns = ns
sc.ingressClass = class

requestTags := []string{"host", "status", "remote_address", "real_ip_address", "remote_user", "protocol", "method", "uri", "upstream_name", "upstream_ip", "upstream_status", "namespace", "ingress", "service"}
requestTags := []string{"host", "status", "protocol", "method", "path", "upstream_name", "upstream_ip", "upstream_status", "namespace", "ingress", "service"}
collectorTags := []string{"namespace", "ingress_class"}

sc.upstreamResponseTime = prometheus.NewHistogramVec(
Expand Down Expand Up @@ -181,12 +177,9 @@ func (sc *SocketCollector) handleMessage(msg []byte) {
requestLabels := prometheus.Labels{
"host": stats.Host,
"status": stats.Status,
"remote_address": stats.RemoteAddress,
"real_ip_address": stats.RealIPAddress,
"remote_user": stats.RemoteUser,
"protocol": stats.Protocol,
"method": stats.Method,
"uri": stats.URI,
"path": stats.Path,
"upstream_name": stats.UpstreamName,
"upstream_ip": stats.UpstreamIP,
"upstream_status": stats.UpstreamStatus,
Expand Down
5 changes: 1 addition & 4 deletions rootfs/etc/nginx/lua/monitor.lua
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@ function _M.encode_nginx_stats()
return cjson.encode({
host = ngx.var.host or "-",
status = ngx.var.status or "-",
remoteAddr = ngx.var.remote_addr or "-",
realIpAddr = ngx.var.realip_remote_addr or "-",
remoteUser = ngx.var.remote_user or "-",
bytesSent = tonumber(ngx.var.bytes_sent) or -1,
protocol = ngx.var.server_protocol or "-",
method = ngx.var.request_method or "-",
uri = ngx.var.uri or "-",
path = ngx.var.location_path or "-",
requestLength = tonumber(ngx.var.request_length) or -1,
requestTime = tonumber(ngx.var.request_time) or -1,
upstreamName = ngx.var.proxy_upstream_name or "-",
Expand Down
17 changes: 4 additions & 13 deletions rootfs/etc/nginx/lua/test/monitor_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,10 @@ describe("Monitor", function()
local nginx_environment = {
host = "testshop.com",
status = "200",
remote_addr = "10.10.10.10",
realip_remote_addr = "5.5.5.5",
remote_user = "admin",
bytes_sent = "150",
server_protocol = "HTTP",
request_method = "GET",
uri = "/admin",
location_path = "/admin",
request_length = "300",
request_time = "60",
proxy_upstream_name = "test-upstream",
Expand All @@ -54,13 +51,10 @@ describe("Monitor", function()
local expected_json_stats = {
host = "testshop.com",
status = "200",
remoteAddr = "10.10.10.10",
realIpAddr = "5.5.5.5",
remoteUser = "admin",
bytesSent = 150.0,
protocol = "HTTP",
method = "GET",
uri = "/admin",
path = "/admin",
Copy link
Contributor

@antoineco antoineco Jun 25, 2018

Choose a reason for hiding this comment

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

Why not location_path? Or the other way around, why location_path in other places and path here.

Copy link
Member Author

@aledbf aledbf Jun 25, 2018

Choose a reason for hiding this comment

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

location_path -> from nginx (variable)
path          -> go side

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah right, nginx_environment vs. expected_json_stats

requestLength = 300.0,
requestTime = 60.0,
upstreamName = "test-upstream",
Expand All @@ -82,7 +76,7 @@ describe("Monitor", function()
remote_user = "francisco",
server_protocol = "HTTP",
request_method = "GET",
uri = "/admin",
location_path = "/admin",
request_time = "60",
proxy_upstream_name = "test-upstream",
upstream_addr = "2.2.2.2",
Expand All @@ -99,13 +93,10 @@ describe("Monitor", function()
local expected_json_stats = {
host = "-",
status = "-",
remoteAddr = "10.10.10.10",
realIpAddr = "5.5.5.5",
remoteUser = "francisco",
bytesSent = -1,
protocol = "HTTP",
method = "GET",
uri = "/admin",
path = "/admin",
requestLength = -1,
requestTime = 60.0,
upstreamName = "test-upstream",
Expand Down