Skip to content

Support for sending connection attributes#737

Closed
Vasfed wants to merge 6 commits intogo-sql-driver:masterfrom
Vasfed:master
Closed

Support for sending connection attributes#737
Vasfed wants to merge 6 commits intogo-sql-driver:masterfrom
Vasfed:master

Conversation

@Vasfed
Copy link
Copy Markdown

@Vasfed Vasfed commented Jan 18, 2018 •
edited
Loading

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
dolmen
dolmen suggested changes Jun 15, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@dolmen dolmen left a comment •
edited
Loading

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Author

Vasfed commented Jun 18, 2018

Rebased onto current master to resolve merge conflicts

@dolmen
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

dolmen commented Jun 1, 2020 •
edited
Loading

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
Copy Markdown

andygrunwald commented Jul 29, 2021

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 andygrunwald mentioned this pull request Jul 31, 2021
Closed
5 tasks
@andygrunwald
Copy link
Copy Markdown

andygrunwald commented Jul 31, 2021

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

pmishchenko-ua pushed a commit to singlestore-labs/go-singlestore-driver that referenced this pull request Apr 11, 2023
Enable sending connection attributes
44d0242 
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

Reviewers

1 more reviewer

@dolmen dolmen dolmen requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

v1.8.0

Development

Successfully merging this pull request may close these issues.

5 participants

@Vasfed @dolmen @andygrunwald @methane @julienschmidt