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/RabbitMQ: Adds connections metricset #6458

Merged
merged 1 commit into from Feb 27, 2018

Conversation

dolftax
Copy link
Contributor

@dolftax dolftax commented Feb 23, 2018

Following metrics are added:
name - The name of the connection with non-ASCII characters escaped as in C.
vhost - Virtual host name with non-ASCII characters escaped as in C.
user - User name.
node - Node name.
channels - The number of channels on the connection.
channel_max - The maximum number of channels allowed on the connection.
frame_max - Maximum permissible size of a frame (in bytes) to negotiate with clients.
type - Type of the connection.
host - Server hostname obtained via reverse DNS, or its IP address if reverse DNS failed or was disabled.
peer_host - Peer hostname obtained via reverse DNS, or its IP address if reverse DNS failed or was not enabled.
peer_port - Peer port.
server_port - Server port.
packet_count - Number of packets sent, received and pending on the connection.
octet_count - Number of octets sent and received on the connection.

Signed-off-by: Jaipradeesh jaipradeesh@gmail.com

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

assert.EqualValues(t, 376, packet_count["receive"])
assert.EqualValues(t, 0, packet_count["pending"])

octet_count := event["octet_count"].(common.MapStr)

Choose a reason for hiding this comment

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

don't use underscores in Go names; var octet_count should be octetCount

assert.EqualValues(t, 131072, event["frame_max"])
assert.EqualValues(t, "network", event["type"])

packet_count := event["packet_count"].(common.MapStr)

Choose a reason for hiding this comment

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

don't use underscores in Go names; var packet_count should be packetCount

mbtest "github.com/elastic/beats/metricbeat/mb/testing"
)

func TestData(t *testing.T) {

Choose a reason for hiding this comment

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

exported function TestData should have comment or be unexported

}, nil
}

func (m *MetricSet) Fetch() ([]common.MapStr, error) {

Choose a reason for hiding this comment

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

exported method MetricSet.Fetch should have comment or be unexported

*helper.HTTP
}

func New(base mb.BaseMetricSet) (mb.MetricSet, error) {

Choose a reason for hiding this comment

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

exported function New should have comment or be unexported

}
}

type MetricSet struct {

Choose a reason for hiding this comment

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

exported type MetricSet should have comment or be unexported

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.

  • Could you add an entry to the CHANGELOG.asciidoc?
  • Can you make Hound-CI happy?

Left few minor comments.

type: keyword
description: >
Server hostname obtained via reverse DNS, or its IP address if reverse DNS failed or was disabled.
- name: peer_host
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this peer.host and `peer.port?

}

func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Experimental("The rabbitmq connection metricset is experimental")
Copy link
Member

Choose a reason for hiding this comment

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

Somehow docs and code do not align here. In the docs it's marked as beta. I assume that has to do with our generator :-( Could you change this to cfgwarn.Beta here?

@ruflin
Copy link
Member

ruflin commented Feb 26, 2018

@jaipradeesh One more thing, you could update the PR description with some more descriptive details?

@ruflin
Copy link
Member

ruflin commented Feb 26, 2018

jenkins, test it

@ruflin
Copy link
Member

ruflin commented Feb 26, 2018

To update the auto generated files you need to run make update.

@dolftax dolftax force-pushed the metricbeat-rabbitmq-connections branch from da78b18 to 28e0c18 Compare February 26, 2018 03:45
@dolftax
Copy link
Contributor Author

dolftax commented Feb 26, 2018

  • Hound's comments on avoid using underscore could be ignored? I followed similar pattern in other metricsets.
  • Marked connections metricset as beta. Should I update node and queue metricsets (not changed in this PR) to Beta as well?
  • Is there a make rule to populate metricbeat/module/rabbitmq/connection/_meta /data.json. I've left it empty now.
  • The following text is contradicting.
➜  beats git:(metricbeat-rabbitmq-connections) cat metricbeat/include/list.go| grep "connection" 
	_ "github.com/elastic/beats/metricbeat/module/rabbitmq/connection"
# From CI build failure logs
metricbeat/include/list.go: needs update
make: *** [check] Error 1

@ruflin

@dolftax dolftax force-pushed the metricbeat-rabbitmq-connections branch from 28e0c18 to aca9c0a Compare February 26, 2018 03:50
@ruflin
Copy link
Member

ruflin commented Feb 26, 2018

  • For the _ in variable names: Please don't use them. Sorry if we missed it in other PR's :-D
  • Lets not touch the other metricsets in this PR. We can handle is in a separate discussion / PR.
  • For the json output have a look at https://github.com/elastic/beats/blob/master/metricbeat/Makefile#L82 You can temporarely adjust the path that it runs only for your metricset
  • For the failing CI: I would expect if you run make update and push all changes again, the CI should be resolved. I can tests it on my end if that does not help.

@dolftax dolftax force-pushed the metricbeat-rabbitmq-connections branch from aca9c0a to a5237e6 Compare February 26, 2018 07:30
@dolftax
Copy link
Contributor Author

dolftax commented Feb 26, 2018

➜  metricbeat git:(metricbeat-rabbitmq-connections) ✗ make generate-json
mkdir -p /home/dolftax/code/src/github.com/jaipradeesh/beats/metricbeat/build
echo "BEAT_STRICT_PERMS=false" > /home/dolftax/code/src/github.com/jaipradeesh/beats/metricbeat/build/test.env
echo "ES_HOST="elasticsearch"" >> /home/dolftax/code/src/github.com/jaipradeesh/beats/metricbeat/build/test.env
echo "ES_PORT=9200" >> /home/dolftax/code/src/github.com/jaipradeesh/beats/metricbeat/build/test.env
echo "ES_USER=beats" >> /home/dolftax/code/src/github.com/jaipradeesh/beats/metricbeat/build/test.env
echo "ES_PASS=testing" >> /home/dolftax/code/src/github.com/jaipradeesh/beats/metricbeat/build/test.env
TESTING_ENVIRONMENT=snapshot-noxpack ES_BEATS=.. docker-compose -p metricbeatsnapshot-noxpack7.0.0-alpha1aca9c0ab94dae095b88dc234732debfcedff35ba  -f docker-compose.yml build 
Building zookeeper
Step 1/2 : FROM jplock/zookeeper:3.4.8
3.4.8: Pulling from jplock/zookeeper
4d06f2521e4f: Pull complete
a3ed95caeb02: Pull complete
8e87dca72717: Pull complete
8e4566461b6e: Pull complete
9c98c5e8ff98: Pull complete
Digest: sha256:de7732696dadb53a52c5f269e8812c5ca696c70849a30faf3a66b44943e6fb73
Status: Downloaded newer image for jplock/zookeeper:3.4.8
 ---> 80e2f8f23445
Step 2/2 : HEALTHCHECK --interval=1s --retries=90 CMD nc -z localhost 2181
 ---> Running in 64520792259a
Removing intermediate container 64520792259a
 ---> d09c06468ec8
..
..
..
---> Running in d4e7357cc2f6
Removing intermediate container d4e7357cc2f6
 ---> 4ed0d234c60e
Successfully built 4ed0d234c60e
Successfully tagged metricbeatsnapshotnoxpack700alpha1aca9c0ab94dae095b88dc234732debfcedff35ba_elasticsearch:latest
Building uwsgi_http
Step 1/8 : FROM python:3.6-alpine
 ---> 29b5ce58cfbc
Step 2/8 : RUN apk add --no-cache --virtual .build-deps gcc libc-dev linux-headers curl
 ---> Using cache
 ---> 4d59bc9624ca
Step 3/8 : RUN pip install --no-cache-dir --trusted-host pypi.python.org uwsgi
 ---> Using cache
 ---> f10497547069
Step 4/8 : WORKDIR /app
 ---> Using cache
 ---> 112222ba543b
Step 5/8 : COPY testdata/app /app
 ---> Using cache
 ---> cb12bcc2f3d5
Step 6/8 : HEALTHCHECK --interval=1s --retries=60 --timeout=10s CMD curl http://localhost:8080/
 ---> Using cache
 ---> 10a6f5ebf6c8
Step 7/8 : EXPOSE 8080 9191 9192
 ---> Using cache
 ---> d75537b62b36
Step 8/8 : CMD [""]
 ---> Using cache
 ---> c1762ccdacc4
Successfully built c1762ccdacc4
Successfully tagged metricbeatsnapshotnoxpack700alpha1aca9c0ab94dae095b88dc234732debfcedff35ba_uwsgi_http:latest
TESTING_ENVIRONMENT=snapshot-noxpack ES_BEATS=.. docker-compose -p metricbeatsnapshot-noxpack7.0.0-alpha1aca9c0ab94dae095b88dc234732debfcedff35ba  -f docker-compose.yml run beat go test -tags=integration github.com/elastic/beats/metricbeat/module/... -data
Creating network "metricbeatsnapshotnoxpack700alpha1aca9c0ab94dae095b88dc234732debfcedff35ba_default" with the default driver
warning: "github.com/elastic/beats/metricbeat/module/..." matched no packages
no packages to test
make: *** [Makefile:83: generate-json] Error 1

@ruflin Help.

@ruflin
Copy link
Member

ruflin commented Feb 26, 2018

I think the problem is that you forked the project under src/github.com/jaipradeesh/beats/metricbeat in the go path instead of src/github.com/elastic/beats/metricbeat. I'm kind of surprised that you managed to run any tests at all for your module? Do you have a parallel fork under the correct directory?

@dolftax
Copy link
Contributor Author

dolftax commented Feb 26, 2018

Parallel fork. Fixing.

Update: http://dpaste.com/3B6W174

@ruflin
Copy link
Member

ruflin commented Feb 26, 2018

I assume make unit works in the metricbeat directory?

@dolftax
Copy link
Contributor Author

dolftax commented Feb 26, 2018

make update is adding _ "github.com/elastic/beats/metricbeat/module/kernel" to metricbeat/include/list.go though module kernel isn't changed as part of this PR.

➜  beats git:(metricbeat-rabbitmq-connections) make update
Generating NOTICE
Get the licenses available from ['./vendor', './metricbeat/vendor', './metricbeat/module/mysql/vendor', './metricbeat/module/postgresql/vendor', './metricbeat/module/vsphere/vendor']
WARNING: No version information found for: github.com/stretchr/objx
Available at /home/dolftax/code/src/github.com/elastic/beats/NOTICE.txt
make[1]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
Updating generated files for libbeat
cat: _meta/beat.yml: No such file or directory
cat: _meta/beat.yml: No such file or directory
make[2]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
make[2]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/libbeat/_meta/kibana/5/index-pattern/libbeat.json
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/libbeat/_meta/kibana/6/index-pattern/libbeat.json
make[1]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
make[1]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/packetbeat'
Updating generated files for packetbeat
make[2]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
make[2]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/packetbeat/_meta/kibana/5/index-pattern/packetbeat.json
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/packetbeat/_meta/kibana/6/index-pattern/packetbeat.json
make[1]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/packetbeat'
make[1]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/filebeat'
find: warning: you have specified the -maxdepth option after a non-option argument -type, but options are not positional (-maxdepth affects tests specified before it as well as those specified after it).  Please specify options before other arguments.

find: warning: you have specified the -mindepth option after a non-option argument -type, but options are not positional (-mindepth affects tests specified before it as well as those specified after it).  Please specify options before other arguments.

Updating generated files for filebeat
make[2]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
make[2]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/filebeat/_meta/kibana/5/index-pattern/filebeat.json
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/filebeat/_meta/kibana/6/index-pattern/filebeat.json
make[1]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/filebeat'
make[1]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/winlogbeat'
Updating generated files for winlogbeat
make[2]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
make[2]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/winlogbeat/_meta/kibana/5/index-pattern/winlogbeat.json
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/winlogbeat/_meta/kibana/6/index-pattern/winlogbeat.json
make[1]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/winlogbeat'
make[1]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/metricbeat'
cp: cannot stat '/home/dolftax/code/src/github.com/elastic/beats/metricbeat/module/kernel/_meta/config.yml': No such file or directory
Updating generated files for metricbeat
make[2]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
make[2]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/metricbeat/_meta/kibana/5/index-pattern/metricbeat.json
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/metricbeat/_meta/kibana/6/index-pattern/metricbeat.json
make[1]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/metricbeat'
make[1]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/heartbeat'
Updating generated files for heartbeat
make[2]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
make[2]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/heartbeat/_meta/kibana/5/index-pattern/heartbeat.json
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/heartbeat/_meta/kibana/6/index-pattern/heartbeat.json
make[1]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/heartbeat'
make[1]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/auditbeat'
Updating generated files for auditbeat
make[2]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
make[2]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/libbeat'
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/auditbeat/_meta/kibana/5/index-pattern/auditbeat.json
-- The index pattern was created under /home/dolftax/code/src/github.com/elastic/beats/auditbeat/_meta/kibana/6/index-pattern/auditbeat.json
make[1]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/auditbeat'
make[1]: Entering directory '/home/dolftax/code/src/github.com/elastic/beats/deploy/kubernetes'
Generating filebeat-kubernetes.yaml
Generating metricbeat-kubernetes.yaml
make[1]: Leaving directory '/home/dolftax/code/src/github.com/elastic/beats/deploy/kubernetes'


➜  beats git:(metricbeat-rabbitmq-connections) ✗ git diff
diff --git a/metricbeat/include/list.go b/metricbeat/include/list.go
index f2163860d..1ab823177 100644
--- a/metricbeat/include/list.go
+++ b/metricbeat/include/list.go
@@ -58,6 +58,7 @@ import (
        _ "github.com/elastic/beats/metricbeat/module/kafka"
        _ "github.com/elastic/beats/metricbeat/module/kafka/consumergroup"
        _ "github.com/elastic/beats/metricbeat/module/kafka/partition"
+       _ "github.com/elastic/beats/metricbeat/module/kernel"
        _ "github.com/elastic/beats/metricbeat/module/kibana"
        _ "github.com/elastic/beats/metricbeat/module/kibana/status"
        _ "github.com/elastic/beats/metricbeat/module/kubernetes"

Update: Removing the above line from metricbeat/include/list.go passes make unit.

@dolftax dolftax force-pushed the metricbeat-rabbitmq-connections branch from a5237e6 to c374fcd Compare February 26, 2018 09:05
@ruflin
Copy link
Member

ruflin commented Feb 26, 2018

There should not be a directory kernel in the modules directory. Can you remove the directory?

@dolftax
Copy link
Contributor Author

dolftax commented Feb 26, 2018

Removed. Generating JSON. It might take a while.

@ruflin
Copy link
Member

ruflin commented Feb 26, 2018

You can temporarely adjust the line to

${DOCKER_COMPOSE} run beat go test -tags=integration github.com/elastic/beats/metricbeat/module/rabbitmq/connection/... -data

or even as following to gather data from a local rabbitmq instance you have running:

go test -tags=integration github.com/elastic/beats/metricbeat/module/rabbitmq/connection/... -data

Following metrics are added:
name - The name of the connection with non-ASCII characters escaped as in C.
vhost - Virtual host name with non-ASCII characters escaped as in C.
user - User name.
node - Node name.
channels - The number of channels on the connection.
channel_max - The maximum number of channels allowed on the connection.
frame_max - Maximum permissible size of a frame (in bytes) to negotiate with clients.
type - Type of the connection.
host - Server hostname obtained via reverse DNS, or its IP address if reverse DNS failed or was disabled.
peer_host - Peer hostname obtained via reverse DNS, or its IP address if reverse DNS failed or was not enabled.
peer_port - Peer port.
server_port - Server port.
packet_count - Number of packets sent, received and pending on the connection.
octet_count - Number of octets sent and received on the connection.

Signed-off-by: Jaipradeesh <jaipradeesh@gmail.com>
@dolftax dolftax force-pushed the metricbeat-rabbitmq-connections branch from c374fcd to 9c133af Compare February 26, 2018 10:15
@dolftax
Copy link
Contributor Author

dolftax commented Feb 26, 2018

@ruflin LGTM. Thanks for the help with onboarding. Let me know.

@ruflin
Copy link
Member

ruflin commented Feb 27, 2018

jenkins, test it

@ruflin ruflin merged commit dae9ddf into elastic:master Feb 27, 2018
@ruflin
Copy link
Member

ruflin commented Feb 27, 2018

@jaipradeesh Thanks for going through this. It's a pleasure to work like this.

@dolftax dolftax deleted the metricbeat-rabbitmq-connections branch February 27, 2018 04:25
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.

None yet

4 participants