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

Wrong template for postgresql statement module #10631

Merged
merged 1 commit into from
Feb 18, 2019
Merged

Wrong template for postgresql statement module #10631

merged 1 commit into from
Feb 18, 2019

Conversation

tehmoon
Copy link
Contributor

@tehmoon tehmoon commented Feb 7, 2019

According to: https://github.com/elastic/beats/blob/master/metricbeat/module/postgresql/statement/data.go the default template casts a long for a ms: float type object.

I don't know if there are other files to change. I suppose that's this yaml file that generates the elasticsearch template. But I could be wrong.

@tehmoon tehmoon requested a review from a team as a code owner February 7, 2019 01:59
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@ruflin ruflin added bug module Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Feb 7, 2019
@ruflin
Copy link
Contributor

ruflin commented Feb 7, 2019

Indeed, these seem to be off. I wonder why our CI did not detect this :-(

To update the dependencies run make update.

@tehmoon
Copy link
Contributor Author

tehmoon commented Feb 7, 2019

Hi! Thank you for the update. I did not checkout the code but I user the online editor on Github.

I will try than and restart the CI.

@tehmoon tehmoon requested a review from a team as a code owner February 8, 2019 01:22
@tehmoon
Copy link
Contributor Author

tehmoon commented Feb 8, 2019

I rebased from master to kick off another build. Not sure why the last one failed.

@kaiyan-sheng
Copy link
Contributor

jenkins, test this

@tehmoon
Copy link
Contributor Author

tehmoon commented Feb 8, 2019

@ruflin Sorry to bother, but it is still failing, let me know if I can help.

@tehmoon
Copy link
Contributor Author

tehmoon commented Feb 12, 2019

@kaiyan-sheng I based the changes on this file https://github.com/elastic/beats/blob/master/metricbeat/module/postgresql/statement/data.go but also from the JSON that the module returns as shown below:

        "statement": {
          "user": {
            "id": 10
          },
          "database": {
            "oid": 175251
          },
          "query": {
            "text": "ROLLBACK",
            "memory": {
              "shared": {
                "hit": 0,
                "read": 0,
                "dirtied": 0,
                "written": 0
              },
              "temp": {
                "read": 0,
                "written": 0
              },
              "local": {
                "hit": 0,
                "read": 0,
                "dirtied": 0,
                "written": 0
              }
            },
            "id": "961354038",
            "rows": 0,
            "time": {
              "min": {
                "ms": 0
              },
              "total": {
                "ms": 0.02900000000000002
              },
              "stddev": {
                "ms": 0.0003768830273792263
              },
              "max": {
                "ms": 0.001
              },
              "mean": {
                "ms": 0.0008285714285714287
              }
            },
            "calls": 35
          }
        }

Should I also change the file you mentioned too?

@kaiyan-sheng
Copy link
Contributor

@tehmoon Thanks for pointing that out! Sorry I totally missed it. Looks like https://github.com/elastic/beats/blob/master/metricbeat/module/postgresql/statement/_meta/data.json needs to be updated. Can you also update the data.json as well please? Thank you!

@tehmoon
Copy link
Contributor Author

tehmoon commented Feb 13, 2019

@kaiyan-sheng no worries at all! I made the change with the json structure that I had directly from the postgresql module. Hopefully the tests will be good!

@kaiyan-sheng kaiyan-sheng merged commit c8b2f1c into elastic:master Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Metricbeat Metricbeat module Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants