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

Add server tag to metrics and service_check #2928

Merged
merged 12 commits into from
Mar 19, 2019

Conversation

ian28223
Copy link
Contributor

@ian28223 ian28223 commented Jan 11, 2019

What does this PR do?

adds pg_instance:<host|host-port> tag to metrics to distinguish which instance the metric came from.

Given a host (e.g. dd-host1) that connects to multiple remote PG instances:

instances:
  - host: /var/run/postgresql/.s.PGSQL.5432
  - host: pg01
    port: 5432
  - host: pg02
    port: 5433

metric submitted from each instance are identical and has no reference to which pg instance they were queried from.

{
  "metric": "postgresql.table.count",  "points": [[ 1547173475, 1]],
  "tags": [
    "schema:public"
  ],
  "host": "dd-host1",
  "type": "gauge", "interval": 0, "source_type_name": "System"
}

With the pg_instance tag, it would look like:

[
  {
    "metric": "postgresql.table.count", "points": [[ 1547173475, 1]],
    "tags": [
      "pg_instance:/var/run/postgresql/.s.PGSQL.5432",
      "schema:public"
    ],
    "host": "dd-host1",
    "type": "gauge", "interval": 0, "source_type_name": "System"
  },
  {
    "metric": "postgresql.table.count", "points": [[ 1547173475, 1]],
    "tags": [
      "pg_instance:pg01-5432",
      "schema:public"
    ],
    "host": "dd-host1",
    "type": "gauge", "interval": 0, "source_type_name": "System"
  },
  {
    "metric": "postgresql.table.count", "points": [[ 1547173475, 1]],
    "tags": [
      "pg_instance:pg02-5433",
      "schema:public"
    ],
    "host": "dd-host1",
    "type": "gauge", "interval": 0, "source_type_name": "System"
  }
]

Motivation

Realized that i could not distinguish which metrics are coming from what instance.

Review checklist

  • PR has a meaningful title or PR has the no-changelog label attached
  • Feature or bugfix has tests
  • Git history is clean
  • If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo
  • If PR adds a configuration option, it has been added to the configuration file.

Additional Notes

Anything else we should know when reviewing?

@ian28223 ian28223 requested review from a team as code owners January 11, 2019 03:08
@codecov-io
Copy link

codecov-io commented Jan 14, 2019

Codecov Report

Merging #2928 into master will increase coverage by 0.4%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2928      +/-   ##
==========================================
+ Coverage   85.44%   85.85%    +0.4%     
==========================================
  Files         676        9     -667     
  Lines       35517      735   -34782     
  Branches     4205      104    -4101     
==========================================
- Hits        30348      631   -29717     
+ Misses       4016       73    -3943     
+ Partials     1153       31    -1122

@gzussa
Copy link
Contributor

gzussa commented Jan 21, 2019

@ChristineTChen @masci Any feedback regarding this PR?

@ian28223
Copy link
Contributor Author

rabased and made some adjustments due to some conflicts

Copy link
Contributor

@gzussa gzussa left a comment

Choose a reason for hiding this comment

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

Nice one!

@gzussa gzussa merged commit 6d1262a into master Mar 19, 2019
@ian28223 ian28223 deleted the ianb/postgres_add_pginstancetag branch March 20, 2019 01:31
@zippolyte zippolyte changed the title [postgres] adds pg_instance tag to metrics adds pg_instance tag to metrics Apr 1, 2019
@zippolyte
Copy link
Contributor

Followed up by #3467

@zippolyte zippolyte changed the title adds pg_instance tag to metrics Add server tag to metrics and service_check Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants