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 license for CCR before attempting to fetch stats #9003

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@ycombinator
Contributor

ycombinator commented Nov 9, 2018

Resolves #8915.

This PR teaches the Fetch() method of the elasticsearch/ccr metricset to first check if the CCR feature is available per the current Elasticsearch license. If it isn't, the metricset logs+reports an actionable error message every minute.

Before this PR, there was no such check so the call to the CCR stats API would simply fail with a 403 error from Elasticsearch if Elasticsearch wasn't using a Trial or Platinum license. That unhelpful 403 error would get logged+reported every 10s (or whatever period the metricset was configured to).

Before this PR

In the metricbeat logs:

screen shot 2018-11-08 at 4 42 28 pm

In the metricbeat-* index:

screen shot 2018-11-08 at 4 42 38 pm

After this PR

In the metricbeat logs:

screen shot 2018-11-08 at 4 35 19 pm

In the metricbeat-* index:

screen shot 2018-11-08 at 4 37 27 pm

@@ -391,3 +391,24 @@ func getSettingGroup(allSettings common.MapStr, groupKey string) (common.MapStr,
return common.MapStr(v), nil
}
// IsCCRAvailable returns whether CCR is available or not, depending on the current license
func IsCCRAvailable(http *helper.HTTP, resetURI string) (isAvailable bool, currentLicense string, err error) {

This comment has been minimized.

@jsoriano

jsoriano Nov 9, 2018

Member

It can be confusing to understand the difference with IsCCRStatsAvailable. We could make this a generic license checker, and/or we could have an only method to check if CCR stats are available in current license AND version.

This comment has been minimized.

@ycombinator

ycombinator Nov 9, 2018

Contributor

I like both your ideas, actually. The version check should probably log+report at the same frequency as the license check. So how about I do this:

  • Make a generic version check function in elasticsearch.go. In fact, I think there is one already.
  • Make a generic license check function in elasticsearch.go.
  • Move isCCRAvailable into ccr.go, since it's really only needed by that metricset. This function would check the license and the version, returning a non-empty log message, if the check fails.

WDYT?

This comment has been minimized.

@jsoriano

jsoriano Nov 9, 2018

Member

I like it 👍

This comment has been minimized.

@ycombinator

ycombinator Nov 9, 2018

Contributor

Refactoring done in 49c422b.

@ycombinator ycombinator added in progress and removed review labels Nov 10, 2018

@ycombinator ycombinator removed the request for review from ruflin Nov 10, 2018

@ycombinator

This comment has been minimized.

Contributor

ycombinator commented Nov 10, 2018

Things aren't quite working correctly after my latest changes so I'm moving this PR out of review for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment