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

Support for sending connection attributes #737

Closed
wants to merge 6 commits into from

Conversation

Vasfed
Copy link

@Vasfed Vasfed commented Jan 18, 2018

Description

This PR adds support for sending connection attributes, which are used for identifying individual connections in mysql. (https://dev.mysql.com/doc/refman/5.7/en/performance-schema-connection-attribute-tables.html)
For example libmysql sets attributes _client_name, _pid, _client_version, _os, _platform. This adds default _client_name=go-mysql-driver

Checklist

  • [+] Code compiles correctly
  • [+] Created tests which fail without the change (if possible)
  • [+] All tests passing
  • [+] Extended the README / documentation, if necessary
  • [+] Added myself / the copyright holder to the AUTHORS file

@Vasfed Vasfed force-pushed the master branch 2 times, most recently from 628d6aa to c6deadb Compare January 18, 2018 19:34
@julienschmidt julienschmidt added this to the v1.5.0 milestone May 22, 2018
Copy link
Contributor

@dolmen dolmen left a comment

Choose a reason for hiding this comment

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

  • This feature requires the performance_schema to be enabled on the server, so this should be clearly documented with the option.
  • Rename the option to connectAttrs as this feature is named connect_attrs in MySQL. Also ConnectAttrs instead of Attributes in struct Config.
  • TestConnectionAttributes should use table INFORMATION_SCHEMA.PROCESSLIST instead of sys.processlist if possible because the sys schema is introduced only in MySQL 5.7.7.

@Vasfed Vasfed force-pushed the master branch 3 times, most recently from 3ee4429 to b2c046d Compare June 15, 2018 17:49
@Vasfed
Copy link
Author

Vasfed commented Jun 15, 2018

Removed sys.processlist from test, renamed option to connectAttrs.
Cannot get tests to pass on travis yet (connection seems not to appear in performance_schema in 5.6, trying to reproduce)

@Vasfed Vasfed changed the title Support for sending connection attributes WIP Support for sending connection attributes Jun 16, 2018
@Vasfed Vasfed changed the title WIP Support for sending connection attributes Support for sending connection attributes Jun 16, 2018
@Vasfed
Copy link
Author

Vasfed commented Jun 16, 2018

@dolmen fixed your suggestions.
Tests were failing on travis because default mysql 5.6 in ci-garnet image has performance_schema=OFF

@Vasfed
Copy link
Author

Vasfed commented Jun 18, 2018

Rebased onto current master to resolve merge conflicts

@dolmen
Copy link
Contributor

dolmen commented Sep 6, 2019

Commit "Fix excessive null-termination for auth data in handshake" do not seem related to this PR. Or is it?

@Vasfed
Copy link
Author

Vasfed commented Sep 9, 2019

@dolmen strictly speaking - it is not, and can be merged alone, but it was included because until it is fixed this PR cannot be merged.
Will look into conflicts - may be it is already fixed in master over all this time

@julienschmidt julienschmidt modified the milestones: v1.5.0, v1.6.0 Sep 24, 2019
@dolmen
Copy link
Contributor

dolmen commented Jun 1, 2020

It would be useful to add tests and make them run under Travis-CI (see file .travis.yml).

Here is a scenario:

  • create a DSN with a program_name
  • connect
  • check SELECT * FROM performance_schema.session_account_connect_attrs that we get values from the DSN.

@julienschmidt julienschmidt modified the milestones: v1.6.0, v1.7.0 Apr 1, 2021
@andygrunwald
Copy link

Hey @dolmen and @Vasfed,
anything I can help with to get this going?

I was aiming to send the connection attribute program_name with my service.

@andygrunwald
Copy link

Hey @dolmen and @Vasfed,
I used this PR, re-integrated it with master and opened a new one: #1241

pmishchenko-ua added a commit to memsql/go-singlestore-driver that referenced this pull request Apr 11, 2023
Summary: In order to be able track Go client usage we enable sending connection attributes as specified in https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_connection_phase_packets_protocol_handshake_response.html (CLIENT_CONNECT_ATTRS). This commit is based on PR go-sql-driver#1241 which is based on PR go-sql-driver#737 (which are still open as of 11.04.2023)

Test Plan:
manual
integration test will be added to the main repo

Reviewers: amakarovych-ua, okramarenko-ua

Reviewed By: okramarenko-ua

Subscribers: engineering-list

Differential Revision: https://grizzly.internal.memcompute.com/D61981
@methane methane modified the milestones: v1.7.0, v1.8.0 May 2, 2023
@methane methane closed this May 25, 2023
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.

5 participants