Skip to content

Conversation

@arielvalentin
Copy link
Contributor

@arielvalentin arielvalentin commented May 31, 2023

MySQL server instances have a logical host name and an instance node names.

This is particularly useful when running in clustered mode where a logical hostname is used to connect to a cluster but you also want to record what specific node is executing your query.

Trilogy exposes the instance node name via @connected_host as well as the configured host name in its connection_options.

This breaking change ensures that the net.peer.name is always set to the logical host name, while the individual instance name is being recorded in a separate attribute.

cc: @cschleiden

MySQL server instances have a logical host name and an instance node names.

This is particularly useful when running in clustered mode where a logical hostname
is used to connect to a cluster but you also want to record what specific node is executing your query.

Trilogy exposes the instance node name via `@connected_host` as well as
the configured `host` name in its `connection_options`.

This breaking change ensures that the `net.peer.name` is always set to the logical host name,
while the individual instance name is being recorded in a separate attribute.
@arielvalentin arielvalentin changed the title feat!: Separate logical MySQL host from connected feat!: Separate logical MySQL host from connected host May 31, 2023

attributes[::OpenTelemetry::SemanticConventions::Trace::DB_NAME] = database_name if database_name
attributes[::OpenTelemetry::SemanticConventions::Trace::PEER_SERVICE] = config[:peer_service] unless config[:peer_service].nil?
attributes['db.mysql.instance.host.name'] = @connected_host if defined?(@connected_host)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea what this be named and I am struggling here.

What do you think @adrianna-chang-shopify @cschleiden ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc: @open-telemetry/specs-semconv-approvers I could use some of your input/feedback here as well.

Copy link
Contributor

@cschleiden cschleiden May 31, 2023

Choose a reason for hiding this comment

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

I see https://github.com/open-telemetry/opentelemetry-ruby/blob/main/semantic_conventions/lib/opentelemetry/semantic_conventions/trace.rb#L71 for something similar for MSSQL

looking at open-telemetry/opentelemetry-specification#3402, maybe
db.mysql.instance.address or db.mysql.address? But db.mysql.instance.host.name would also work for me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice find!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@cschleiden cschleiden 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 this change!


attributes[::OpenTelemetry::SemanticConventions::Trace::DB_NAME] = database_name if database_name
attributes[::OpenTelemetry::SemanticConventions::Trace::PEER_SERVICE] = config[:peer_service] unless config[:peer_service].nil?
attributes['db.mysql.instance.host.name'] = @connected_host if defined?(@connected_host)
Copy link
Contributor

@cschleiden cschleiden May 31, 2023

Choose a reason for hiding this comment

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

I see https://github.com/open-telemetry/opentelemetry-ruby/blob/main/semantic_conventions/lib/opentelemetry/semantic_conventions/trace.rb#L71 for something similar for MSSQL

looking at open-telemetry/opentelemetry-specification#3402, maybe
db.mysql.instance.address or db.mysql.address? But db.mysql.instance.host.name would also work for me

@arielvalentin arielvalentin marked this pull request as ready for review June 1, 2023 14:08
@arielvalentin arielvalentin marked this pull request as draft June 1, 2023 14:09
@arielvalentin arielvalentin self-assigned this Jun 1, 2023
@arielvalentin arielvalentin marked this pull request as ready for review June 1, 2023 14:12
Copy link
Contributor

@ericmustin ericmustin left a comment

Choose a reason for hiding this comment

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

lgtm, may wanna confirm with known users of the instrumentation (cc @adrianna-chang-shopify ) that this works for them.

@ericmustin
Copy link
Contributor

ah @arielvalentin looks like a couple typos, my apols on the early approval, that aside this lgtm

Copy link
Contributor

@plantfansam plantfansam left a comment

Choose a reason for hiding this comment

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

Love this! As a trilogy instrumentation user, we can handle the churn in attr name 😄.

@arielvalentin arielvalentin merged commit f6df818 into open-telemetry:main Jun 1, 2023
@arielvalentin arielvalentin deleted the trilogy-db-cluster-name branch June 2, 2023 14:10
arielvalentin added a commit to arielvalentin/semantic-conventions that referenced this pull request Sep 26, 2023
At GitHub we run MySQL in multi-node clusters and are often interested in
knowing the hostname of a node that is executing a query or command.

In order to acheive this we added an attribute to the Trilogy instrumentation
named `db.mysql.instance.address` which is populated with the hostname of the node:

<open-telemetry/opentelemetry-ruby-contrib#487>

This change adds the attribute to the database model as a MySQL specific connection attribute.
arielvalentin added a commit to arielvalentin/semantic-conventions that referenced this pull request Nov 15, 2023
At GitHub we run MySQL in multi-node clusters and are often interested in
knowing the hostname of a node that is executing a query or command.

In order to acheive this we added an attribute to the Trilogy instrumentation
named `db.mysql.instance.address` which is populated with the hostname of the node:

<open-telemetry/opentelemetry-ruby-contrib#487>

This change adds the attribute to the database model as a MySQL specific connection attribute.
arielvalentin added a commit to arielvalentin/semantic-conventions that referenced this pull request Nov 19, 2023
At GitHub we run MySQL in multi-node clusters and are often interested in
knowing the hostname of a node that is executing a query or command.

In order to acheive this we added an attribute to the Trilogy instrumentation
named `db.mysql.instance.address` which is populated with the hostname of the node:

<open-telemetry/opentelemetry-ruby-contrib#487>

This change adds the attribute to the database model as a MySQL specific connection attribute.
arielvalentin added a commit to arielvalentin/semantic-conventions that referenced this pull request Nov 19, 2023
At GitHub we run MySQL in multi-node clusters and are often interested in
knowing the hostname of a node that is executing a query or command.

In order to acheive this we added an attribute to the Trilogy instrumentation
named `db.mysql.instance.address` which is populated with the hostname of the node:

<open-telemetry/opentelemetry-ruby-contrib#487>

This change adds the attribute to the database model as a MySQL specific connection attribute.
@github-actions github-actions bot mentioned this pull request Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants