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] Add ability to collect client-provided name for rabbitmq connections #12852

Merged
merged 4 commits into from
Jul 12, 2019
Merged

[Metricbeat] Add ability to collect client-provided name for rabbitmq connections #12852

merged 4 commits into from
Jul 12, 2019

Conversation

kaiyan-sheng
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng commented Jul 10, 2019

This PR is to add collection for Client-provided names in rabbitmq connections.
Screen Shot 2019-07-10 at 11 54 30 AM

RabbitMQ 3.6.5 added the facility for the connecting client to report a friendly name string value to identify a connection for management purposes. This is strictly an identifier and, as it is client-reported, it cannot be relied upon for anything other than weak identification of connections.

closes #12851

How to test it:

Bring up rabbitmq by running: rabbitmq-server
How to send and receive messgaes: https://www.rabbitmq.com/tutorials/tutorial-one-go.html
How to start a connection with client-provided name:

cfg := amqp.Config{
		Properties: amqp.Table{
			"connection_name": "ThisIsMyName",
		},
	}
conn, err := amqp.DialConfig("amqp://guest:guest@localhost:5672/", cfg)
failOnError1(err, "Failed to connect to RabbitMQ")
defer conn.Close()

ch, err := conn.Channel()

@kaiyan-sheng kaiyan-sheng requested review from a team as code owners July 10, 2019 18:12
@kaiyan-sheng kaiyan-sheng self-assigned this Jul 10, 2019
@kaiyan-sheng kaiyan-sheng added Team:Integrations Label for the Integrations team Metricbeat Metricbeat labels Jul 10, 2019
Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

Left a single comment. Tell me WDYT :)

"frame_max": c.Int("frame_max"),
"type": c.Str("type"),
"name": c.Str("name"),
"client_provided_name": c.Str("client_properties.connection_name"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to call this client_provided.name or event something like client.name. The reason is that probably we can enrich this with more fields in a near future (or maybe not) an, if so, we probably won't like to have
client_provided_name
client_provided_tag
client_provided_label

Copy link
Member

Choose a reason for hiding this comment

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

Take into account that this is a name for the connection, so +1 to something like client_provided.name, but don't use client.name, it would look like the name of the client.
We may also keep the original client_properties naming, so if there are more properties in the future we store them as:

rabbitmq.connection.client_properties.name
rabbitmq.connection.client_properties.tag
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah good point! Thanks @jsoriano and @sayden! I think I'm leaning towards using rabbitmq.connection.client_provided.name just to make it more consistent with the rabbitmq UI 😬

@odacremolbap
Copy link
Contributor

tested, ok on my side.

for anyone testing integrations:

    cfg := amqp.Config{Properties: amqp.Table{"connection_name": "mb-rabbitmq-test"}}
    conn, err := amqp.DialConfig("amqp://guest:guest@localhost:5672/", cfg)

@kaiyan-sheng
Copy link
Contributor Author

@odacremolbap Thanks for pointing that out. I need to update the PR description with how to test it.

@kaiyan-sheng kaiyan-sheng merged commit 52aad27 into elastic:master Jul 12, 2019
@kaiyan-sheng kaiyan-sheng deleted the add_connection_name branch July 12, 2019 03:49
@andresrc andresrc added test-plan Add this PR to be manual test plan v7.4.0 labels Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Metricbeat Metricbeat Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan v7.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Metricbeat] Collect client-provided name from rabbitmq connections
5 participants