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

[metricbeat] Migrate nginx/substatus to reporterv2 with error #13518

Conversation

fearful-symmetry
Copy link
Contributor

@fearful-symmetry fearful-symmetry commented Sep 5, 2019

See #11374

There was a community PR for this, but it's been abandoned so I'm just doing it myself. This one is a bit odd, since it requires changes to some other metricbeat tests, which will look for error strings.

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

It LGTM, added a small suggestion.

@@ -48,8 +48,8 @@ func TestFetch(t *testing.T) {
func TestData(t *testing.T) {
service := compose.EnsureUp(t, "nginx")

f := mbtest.NewReportingMetricSetV2(t, getConfig(service.Host()))
if err := mbtest.WriteEventsReporterV2(f, t, ""); err != nil {
Copy link
Member

@jsoriano jsoriano Sep 5, 2019

Choose a reason for hiding this comment

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

Or even better 🙂

f := mbtest.NewFetcher(t, getConfig(service.Host()))
f.WriteEvents(t, "")

@fearful-symmetry
Copy link
Contributor Author

jenkins, test this

@fearful-symmetry
Copy link
Contributor Author

I don't think the CI failures I'm seeing are related.

@fearful-symmetry
Copy link
Contributor Author

jenkins, test this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Integrations Label for the Integrations team v7.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants