Skip to content

/ mysql

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

Jump to bottom

Support for sending connection attributes #737

Open
wants to merge 6 commits into
base: master
from
Open

Support for sending connection attributes #737

wants to merge 6 commits into from

Conversation

@Vasfed
Copy link

@Vasfed Vasfed commented Jan 18, 2018 •
edited

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 Vasfed:master branch 2 times, most recently from 628d6aa to c6deadb Jan 18, 2018
@julienschmidt julienschmidt added this to the v1.5.0 milestone May 22, 2018
@dolmen
dolmen suggested changes Jun 15, 2018
View changes
Copy link
Contributor

@dolmen dolmen left a comment •
edited

  • 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 Vasfed:master branch 3 times, most recently from 3ee4429 to b2c046d Jun 15, 2018
@Vasfed
Copy link
Author

@Vasfed 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 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 added 6 commits Jan 18, 2018
@Vasfed
Support for sending connection attributes
71151ad
@Vasfed
Rename connectionAttributes to connectAttrs and mention performance_s…
9057689 
…chema in readme
@Vasfed
Fix formatting
71a1948
@Vasfed
Fix excessive null-termination for auth data in handshake
7038d92
@Vasfed
Use performance_schema.session_connect_attrs in test
151e4dc
@Vasfed
Skip connectAttrs test if performance_schema is disabled
Loading status checks…
26b97ba
@Vasfed Vasfed force-pushed the Vasfed:master branch from e03b778 to 26b97ba Jun 18, 2018
@Vasfed
Copy link
Author

@Vasfed Vasfed commented Jun 18, 2018

Rebased onto current master to resolve merge conflicts
@dolmen
Copy link
Contributor

@dolmen 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 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 dolmen commented Jun 1, 2020 •
edited

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dolmen dolmen

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
v1.6.0
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@Vasfed @dolmen @julienschmidt
You can’t perform that action at this time.