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 license expiry date zero value #14591

Merged
merged 8 commits into from
Nov 19, 2019

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Nov 18, 2019

Resolves #14541.

Users using Metricbeat to monitor an Elasticsearch cluster with a Basic license will not be able to access that cluster in the Stack Monitoring UI. This is because the elasticsearch/cluster_stats metricset is recording a Basic license's expiration date as 0, which the UI considers to a sentinel value for an expired license.

This PR fixes this bug by not recording the expiration date field at all, if its value is 0. This mimics what internal monitoring collection is doing.

Testing this PR

  1. Spin up an Elasticsearch node with a Basic license.
  2. Build Metricbeat with this PR.
    cd $GOPATH/src/github.com/elastic/beats
    mage build
    
  3. Enable the elasticsearch-xpack Metricbeat module.
    ./metricbeat modules enable elasticsearch-xpack
    
  4. Run Metricbeat.
    ./metricbeat -e
    
  5. Verify that the latest document in .monitoring-es-*-mb-* of type: cluster_stats does not record the license expiration date in ms.
    http://localhost:9200/.monitoring-*-mb-*/_search?q=type:cluster_stats&sort=timestamp:desc&size=1&filter_path=hits.hits._source.license
    

@elasticmachine
Copy link
Collaborator

Pinging @elastic/stack-monitoring (Stack monitoring)

Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

I tested this functionality, and it LGTM!

POST .monitoring-es-7-mb-2019.11.18/_search?filter_path=hits.hits._source.license
{
  "size": 1,
  "sort": [
    {
      "timestamp": {
        "order": "desc"
      }
    }
  ],
  "query": {
    "term": {
      "type": {
        "value": "cluster_stats"
      }
    }
  }
}

yields:

{
  "hits" : {
    "hits" : [
      {
        "_source" : {
          "license" : {
            "status" : "active",
            "issued_to" : "elasticsearch",
            "issue_date_in_millis" : 1573845929339,
            "max_nodes" : 1000,
            "issue_date" : "2019-11-15T19:25:29.339Z",
            "start_date_in_millis" : -1,
            "issuer" : "elasticsearch",
            "cluster_needs_tls" : false,
            "id" : "4c776e8e-8795-43f4-b74e-f0b6c1303e44",
            "type" : "basic"
          }
        }
      }
    ]
  }
}

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>
@@ -82,7 +82,7 @@ func (m *MetricSet) Fetch(r mb.ReporterV2) error {
if ccrUnavailableMessage != "" {
if time.Since(m.lastCCRLicenseMessageTimestamp) > 1*time.Minute {
m.lastCCRLicenseMessageTimestamp = time.Now()
m.Logger().Warn(ccrUnavailableMessage)
m.Logger().Debug(ccrUnavailableMessage)
Copy link
Contributor Author

@ycombinator ycombinator Nov 18, 2019

Choose a reason for hiding this comment

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

I made this change for two reasons:

  1. It's consistent with a similar logging behavior seen in the elasticsearch/enrich metricset:

    m.Logger().Debug(enrichUnavailableMessage)

  2. By changing the log level to debug here (instead of warn), this check in the system test doesn't fail:

    self.assert_no_logged_warnings()

@ycombinator
Copy link
Contributor Author

@kaiyan-sheng Thanks for the review. I had to make a couple of changes to the PR since your review to make the system test pass. Would you mind re-reviewing, please? Thanks!

Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for explaining the change from warn to debug.

@ycombinator
Copy link
Contributor Author

Travis CI is green and Jenkins CI failures are unrelated. Merging.

@ycombinator ycombinator merged commit f0b9500 into elastic:master Nov 19, 2019
@ycombinator ycombinator deleted the bugfix-mb-es-license-expiry branch November 19, 2019 01:34
ycombinator added a commit to ycombinator/beats that referenced this pull request Nov 19, 2019
* Check license expiry date zero value

* Adding check to system test

* Refactoring: moving fix to better location in code

* Adding CHANGELOG entry

* Fixing typo

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>

* Change CCR log message to debug

* Start basic license
ycombinator added a commit to ycombinator/beats that referenced this pull request Nov 19, 2019
* Check license expiry date zero value

* Adding check to system test

* Refactoring: moving fix to better location in code

* Adding CHANGELOG entry

* Fixing typo

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>

* Change CCR log message to debug

* Start basic license
ycombinator added a commit that referenced this pull request Nov 19, 2019
* Check license expiry date zero value (#14591)

* Check license expiry date zero value

* Adding check to system test

* Refactoring: moving fix to better location in code

* Adding CHANGELOG entry

* Fixing typo

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>

* Change CCR log message to debug

* Start basic license

* Fixing up CHANGELOG
ycombinator added a commit that referenced this pull request Nov 19, 2019
* Check license expiry date zero value (#14591)

* Check license expiry date zero value

* Adding check to system test

* Refactoring: moving fix to better location in code

* Adding CHANGELOG entry

* Fixing typo

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>

* Change CCR log message to debug

* Start basic license

* Fixing up CHANGELOG
ycombinator added a commit to ycombinator/beats that referenced this pull request Dec 18, 2019
* Check license expiry date zero value

* Adding check to system test

* Refactoring: moving fix to better location in code

* Adding CHANGELOG entry

* Fixing typo

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>

* Change CCR log message to debug

* Start basic license
ycombinator added a commit that referenced this pull request Dec 23, 2019
* Check license expiry date zero value (#14591)

* Check license expiry date zero value

* Adding check to system test

* Refactoring: moving fix to better location in code

* Adding CHANGELOG entry

* Fixing typo

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>

* Change CCR log message to debug

* Start basic license

* Fixing up CHANGELOG

* Adding blank line

* Fixing up code

* Updating license URLs

* Fixing up CHANGELOG

* Adding existence check for type field in doc
@ycombinator ycombinator added v6.8.7 and removed needs_backport PR is waiting to be backported to other branches. labels Jan 15, 2020
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…4604)

* Check license expiry date zero value (elastic#14591)

* Check license expiry date zero value

* Adding check to system test

* Refactoring: moving fix to better location in code

* Adding CHANGELOG entry

* Fixing typo

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>

* Change CCR log message to debug

* Start basic license

* Fixing up CHANGELOG
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.

Metricbeat collection of ES monitoring data is inconsistent for the license field
4 participants