-
Notifications
You must be signed in to change notification settings - Fork 458
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
Add stan package #532
Add stan package #532
Conversation
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please rebase it against the master branch? I pushed a new test runner for checking presence of package assets. It will verify if the package can be installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, all tests are passing including assets: https://beats-ci.elastic.co/blue/organizations/jenkins/Ingest-manager%2Fintegrations/detail/PR-532/13/tests
I left some comments.
|
||
## Compatibility | ||
|
||
The STAN package is tested with Stan 0.15.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: missing dot at the end
FROM golang:1.13-alpine3.11 AS build-env | ||
RUN apk --no-cache add build-base git mercurial gcc | ||
RUN cd src && go get -d github.com/nats-io/stan.go/ | ||
RUN cd src/github.com/nats-io/stan.go/examples/stan-bench && git checkout tags/v0.5.2 && go build . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I wonder if there isn't a prebuilt image for this, but let's leave it if there isn't
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't find anything with a quick search maintained by NATS project and I would avoid using a random third party custom image (if any) so better to handle it on our own. In the future these images will be cached, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM! We'll cache image when it becomes the problem. So far image building doesn't take much time.
#while true; do /stan-bench -np 10 -ns 10 -n 1000000000 -ms 1024 bar; done & | ||
|
||
# Make sure the container keeps running | ||
tail -f /dev/null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this line necessary considering the while true
above?
} | ||
] | ||
}, | ||
"title": " Bytes Timeline [Logs STAN]", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: unnecessary space
} | ||
} | ||
}, | ||
"title": " Bytes Timeline [Logs STAN]", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: actually this pattern is repeated in few places:
"title": "<space>
packages/stan/manifest.yml
Outdated
kibana.version: ">=7.11.0" | ||
screenshots: | ||
- src: /img/metricbeat-stan-overview.png | ||
title: Metricbeat STAN Dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics
packages/stan/manifest.yml
Outdated
size: 1829x447 | ||
type: image/png | ||
- src: /img/filebeat-stan-overview.png | ||
title: Filebeat STAN Dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logs, not filebeat
Signed-off-by: ChrsMark <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please wait for green. If it passes, fee free to merge this PR. LGTM!
packages/stan/manifest.yml
Outdated
@@ -17,11 +17,11 @@ conditions: | |||
kibana.version: ">=7.11.0" | |||
screenshots: | |||
- src: /img/metricbeat-stan-overview.png |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think you can fix filenames too.
Signed-off-by: ChrsMark <[email protected]>
What does this PR do?
This PR adds
stan
metricset.Still need to add a dashboard for
log
datastream.Checklist
How to test this PR locally
Test the service manually using
elastic-package
:/tmp/service_logs/stan.log*
for logs andelastic-package-service_stan_1:8222
for metrics.Related issues
Screenshots