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

Check Kibana version for stats API availability #7697

Merged
merged 6 commits into from
Jul 24, 2018
Merged

Check Kibana version for stats API availability #7697

merged 6 commits into from
Jul 24, 2018

Conversation

ycombinator
Copy link
Contributor

Metricbeat will now exit with a helpful error message if the user is running against a version of Kibana that doesn't yet have the stats API implemented in it.

To test this PR

  1. Run Elasticsearch 6.3.0.

    docker run --rm --name elasticsearch docker.elastic.co/elasticsearch/elasticsearch:6.3.0
    
  2. Run Kibana 6.3.0.

    docker run --rm --name kibana --publish 5601:5601 --link elasticsearch docker.elastic.co/kibana/kibana:6.3.0
    
  3. Using this PR, enable the kibana module in metricbeat.

    metricbeat module enable kibana
    
  4. Modify modules.d/kibana.yml and enable the stats metricset.

  5. Run metricbeat.

    metricbeat -e
    
  6. Verify that metricbeat exits with a helpful error message about the Kibana stats API not being available.

}

var data common.MapStr
json.Unmarshal(content, &data)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check the error here.

defer http.SetURI(currentURI) // Reset after this request

// Parse the URI to replace the path
u, _ := url.Parse(currentURI)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we check the error here?

@@ -49,6 +58,13 @@ type MetricSet struct {
xPackEnabled bool
}

func isKibanaStatsAPIAvailable(kibanaVersion string) bool {
currenVersion := semver.New(kibanaVersion)
Copy link
Contributor

Choose a reason for hiding this comment

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

We have an internal version package which could be used here.

@ycombinator
Copy link
Contributor Author

@ruflin Addressed review feedback and squashed to single commit.

return "", err
}

return version.(string), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

What if version.number does not contain a string?

Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

WFG

@ruflin ruflin merged commit 86dc973 into elastic:master Jul 24, 2018
@ruflin
Copy link
Contributor

ruflin commented Jul 25, 2018

@ycombinator Can you create a follow up PR that cleans up the FIXME part?

@ycombinator
Copy link
Contributor Author

ycombinator commented Jul 25, 2018

Can you create a follow up PR that cleans up the FIXME part?

Yup, filed issues to track these FIXMEs:

#7732 and #7733.

@ycombinator ycombinator deleted the metricbeat/kibana/stats/check-version branch July 25, 2018 11:06
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.

2 participants