Skip to content

[Metricbeat] Add STAN (NATS streaming) module#14839

Merged
ChrsMark merged 36 commits intoelastic:masterfrom
ChrsMark:stan_mb_module
Dec 13, 2019
Merged

[Metricbeat] Add STAN (NATS streaming) module#14839
ChrsMark merged 36 commits intoelastic:masterfrom
ChrsMark:stan_mb_module

Conversation

@ChrsMark
Copy link
Copy Markdown
Member

@ChrsMark ChrsMark commented Nov 28, 2019 •
edited
Loading

This PR cherry-picks the commits of @devon-kim
from 7.2...devon-kim:ops-1844-stan-metricbeat so as to be merged to beats/master as part of #14629.

cc: @exekias

Manual testing

  1. Build the Docker image: docker build -t stan_test . (run this command under x-pack/metricbeat/module/stan/_meta)
  2. Start STAN server: docker run -p 8222:8222 stan_test
  3. Enable the module and run Metricbeat.
    (Note that default fetching period is 60secs, so this might should be changed for the testing)

@ChrsMark ChrsMark added in progress Pull request is currently in progress. module Metricbeat Metricbeat [zube]: In Progress labels Nov 28, 2019
@ChrsMark ChrsMark requested a review from a team as a code owner November 28, 2019 11:38
@ChrsMark ChrsMark self-assigned this Nov 28, 2019
houndci-bot
houndci-bot reviewed Nov 28, 2019
View reviewed changes
houndci-bot
houndci-bot reviewed Nov 28, 2019
View reviewed changes
@ghost
Copy link
Copy Markdown

ghost commented Nov 28, 2019

Hi @ChrsMark, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in your Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

@ChrsMark ChrsMark added the Team:Integrations Label for the Integrations team label Nov 28, 2019
houndci-bot
houndci-bot reviewed Nov 28, 2019
View reviewed changes
houndci-bot
houndci-bot reviewed Nov 28, 2019
View reviewed changes
houndci-bot
houndci-bot reviewed Nov 28, 2019
View reviewed changes
@ChrsMark ChrsMark force-pushed the stan_mb_module branch 2 times, most recently from f5e9331 to 4d9ccfc Compare November 28, 2019 12:50
@ChrsMark ChrsMark force-pushed the stan_mb_module branch 2 times, most recently from e4b2602 to f9ccafe Compare December 2, 2019 14:52
fearful-symmetry
fearful-symmetry reviewed Dec 2, 2019
View reviewed changes
metricbeat/module/stan/channels/channels_test.go Outdated
Copy link
Copy Markdown
Contributor

@fearful-symmetry fearful-symmetry Dec 2, 2019

Choose a reason for hiding this comment

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

Might be worth moving some of the setup to another function?

metricbeat/module/stan/channels/data.go Outdated
Copy link
Copy Markdown
Contributor

@fearful-symmetry fearful-symmetry Dec 2, 2019

Choose a reason for hiding this comment

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

We're checking the same error twice here, it looks like?

metricbeat/module/stan/channels/data.go Outdated
Copy link
Copy Markdown
Contributor

@fearful-symmetry fearful-symmetry Dec 2, 2019

Choose a reason for hiding this comment

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

There's been some debate about this. The only time r.Event will return false is when the underlying channel is closed and metricbeat is shutting down. I believe we decide that if we check this in a loop (which we should) It shouldn't be reported as an error and we should just return.

devon-kim and others added 6 commits December 2, 2019 17:27
@devon-kim @ChrsMark
Add STAN metricbeat module (#1)
10bca48 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@devon-kim @ChrsMark
bool -> boolean for field declaration
3b78c08 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@devon-kim @ChrsMark
bool != boolean
8ce67ae 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
fmt update
7ffee37 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Add comments on exported structs
febbd04 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Fix dahsboard
79743d4 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
kaiyan-sheng
kaiyan-sheng reviewed Dec 2, 2019
View reviewed changes
@ChrsMark
improvements
c190bb1 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Copy Markdown
Member Author

ChrsMark commented Dec 11, 2019

SGTM

not requesting changes but commenting for your consideration

Thanks for the suggestions @odacremolbap ! Updates were pushed.

@ChrsMark ChrsMark added test-plan Add this PR to be manual test plan v7.6.0 labels Dec 11, 2019
@ChrsMark
Merge remote-tracking branch 'upstream/master' into stan_mb_module
75dd7c2
@ChrsMark
make update
c582e48 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Move python test to module folder
bd20e7e 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
fix location of Dockefile
33e986e 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
keep output active so as to prevent Travis timeout
e20c930 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Copy Markdown
Member Author

ChrsMark commented Dec 12, 2019 •
edited
Loading

pinging @jsoriano @exekias about the very latest commit. This was needed so as to prevent Travis from giving a timeout while waiting for output. Do you think this could be included? Or any other ideas on how we could unblock this PR?

@exekias
Copy link
Copy Markdown
Contributor

exekias commented Dec 12, 2019

As discussed offline, let's try to make mage log the output of go test to avoid this problem

@ChrsMark
Make use travis_wait
82addd4 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Enable testing output
a4bd16d 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Copy Markdown
Member Author

ChrsMark commented Dec 13, 2019 •
edited
Loading

Problems with Travis were resolved with the change @jsoriano proposed in #15083. The changes we applied here directly. Merging this.

@ChrsMark ChrsMark merged commit 10c741a into elastic:master Dec 13, 2019
@ChrsMark ChrsMark mentioned this pull request Dec 13, 2019
Closed
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Dec 13, 2019
@ChrsMark
[Metricbeat] Add STAN (NATS streaming) module (elastic#14839)
c315812 
(cherry picked from commit 10c741a)
@ChrsMark ChrsMark mentioned this pull request Dec 13, 2019
Merged
ChrsMark added a commit that referenced this pull request Dec 16, 2019
@ChrsMark
Cherry-pick #14839 to 7.x: [Metricbeat] Add STAN (NATS streaming) mod…
00c3c76 
…ule (#15092)
@odacremolbap odacremolbap self-assigned this Jan 16, 2020
@odacremolbap odacremolbap added the test-plan-ok This PR passed manual testing label Jan 20, 2020
@jsoriano jsoriano mentioned this pull request Jan 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fearful-symmetry fearful-symmetry fearful-symmetry left review comments

+5 more reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@kaiyan-sheng kaiyan-sheng kaiyan-sheng left review comments

@devon-kim devon-kim devon-kim left review comments

@odacremolbap odacremolbap odacremolbap approved these changes

@mtojek mtojek mtojek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@odacremolbap odacremolbap

Labels

Metricbeat Metricbeat module review Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-ok This PR passed manual testing v7.6.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@ChrsMark @devon-kim @exekias @houndci-bot @fearful-symmetry @odacremolbap @mtojek @kaiyan-sheng @andresrc