Connection logging #39
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- implement prompt_user_config() based on EsOdbcDsnEdit() entry point.; - implement the callbacks on the GUI actions; - implement DSN bundle serializing to 00-list (which is going to be the way to send/receive the DSN at interfacing point). - start a unit test on the 'dsn' module.
- check for minimum requirements when configuring a DSN (it's name and server location); also perform a DBC configuration with the data, to prevent incomplete data be saved; the connection could still fail later, though, this is just a consistency check with no reachability checks. - perform a connection test in handler when prompting from SQLDriverConnect(), to fail there, rather than later in the function when the error message will not be forwarded back to the user.
- implement prompt_user_config() based on EsOdbcDsnEdit() entry point.; - implement the callbacks on the GUI actions; - implement DSN bundle serializing to 00-list (which is going to be the way to send/receive the DSN at interfacing point). - start a unit test on the 'dsn' module.
- check for minimum requirements when configuring a DSN (it's name and server location); also perform a DBC configuration with the data, to prevent incomplete data be saved; the connection could still fail later, though, this is just a consistency check with no reachability checks. - perform a connection test in handler when prompting from SQLDriverConnect(), to fail there, rather than later in the function when the error message will not be forwarded back to the user.
…-sql-odbc into feature/gui_config
- documentation indicates that last parameter returns number of written bytes, but it's actually characters. Also: - rename 'list00' paramter to 'dsn_str' since the GUI-driver interface will now pass connection strings (since .net framework has a parser for it); - add more DSN validation and fill in the default values for attributs not provided by user.
- Add an editor that is able to edit the DSN as a connection string. - Add it into CMakeLists.txt as a "custom" target, that is also target-aware. - change the build.bat to "recognize" on subsequent invocations the initially set build type.
- add the editor DLLs into the unit tests running dir
…-sql-odbc into feature/gui_config
In count mode (output string is NULL), the function is supposed to return the min size of a buffer that could accomodate the connection string. This commit fixes it's calculation. A unittest was added for it, too.
- place all functionality in one function Also: - fix registry writing for update and new DSN addition.
- bubble up more error messages from DSN validation through DBC configuration; - connection testing and DSN saving callbacks now use the DSN validation function to do most of their work; Also: - add []-framing to IPv6 addresses in URLs - make connection string printing more robust in case destination buffer is too small for the result.
Disable name and description on edit - fixes #28
…-sql-odbc into feature/gui_config
…tname / name. Partially fixes #29 Don't clear the connection string Builder on form close (fixes connection bug)
Mask password box. Refactor StripBraces() method. Added [STAThread] attribute to the factory method.
- now UTF8 - remove BOM
- this fixes the file chooser issue; - DSN form now rendered with .ShowDialog() to prevent calling app allow the user to re-invoke the bridge (which would fail, it running as an STA) fixes #25
…-sql-odbc into feature/gui_config
- in case STA init'ing fails, create a new thread for the form; - also, make the window handler a static member of the binding class. closes #25
… the user that the operation is underway and that they should wait. Fixes #30
…-sql-odbc into feature/gui_config
- this will be used to enable/disable the per-dbc logging - the tick-box in the GUI is now left unchecked if the corresponding parameter is missing from the connection string.
- use DRIVER_BUILD macro, which signals a driver build
- add Elastic's license to all source files
- rename EsOdbcDsnBinding.cpp to EsOdbcDsnBinding.cc, to be consistent with the rest of the repo (.cc test files)
With this commit it's now possible to have one log file per connection.
There is now still onen type of logger, but two types of instaces (each
instance writes in one dedicated log file):
- per process: each process attaching to the driver DLL gets one
instance.
- per connection: each instantiated connection can get one instance (if
its config parameters ask for one)
The two types can coexist. The former is configured through an
environment variable, the later through the DSN parameters.
The handle logging macros (INFOH, DBGH etc.) will use the per-connection
logger, if the respective handles are associated with a connection with
separate logging enabled. If that's not enabled, the "global",
per-process logger will be used.
The non-handle logging macros (INFO, DBG etc.) can still only make use
of the per-process logger.
This commit also fixes the tracing (=logging of calls at driver input
and output) format specifier for wide char strings with precision ('W')
and adds new specifiers for SQL[U]LEN.
- snprintf won't fail in case the formatted string doesn't fit the given buffer (unlike swprintf), so the "buffer too small" condition needs explicit checks.
- in some cases (like select fields count is larger than an index limit) the error reason given in "error"."root_cause" field is different and more relevant than the "error"."reason" field; => if available, choose that message in the first element in the "root_cause" array to post as failure diagnostic.
- replace the password attribute value with a substitute - the places where these are part of a string are commented out for release builds.
- signal through the provided input handle when a API function is not implemented.
- add the missing attribute handling for a connection handle; most are read-only attributes either marking an unsupported feature or informing the caller on how the driver/DSN work.
- convert logging lines to handle-logging (INFOH() etc.) where is possible, so that those messages end up in per-connection log files (which can be provided for troubleshooting purposes).
- ERR -> ERRH
- use an atomic increment for log file stamping, instead of a mutex; - correct the mixing of narrow and wide chars when escaping URL chars to be used in logging file name.
droberts195
reviewed
Oct 24, 2018
| @@ -671,7 +660,7 @@ static BOOL config_dbc_logging(esodbc_dbc_st *dbc, esodbc_dsn_attrs_st *attrs) | |||
| cnt = swprintf(ident.str, ident.cnt, | |||
| WPFWP_LDESC "_" WPFWP_LDESC "_" "%d-%u", | |||
| LWSTR(&attrs->server), LWSTR(&attrs->port), | |||
| GetCurrentProcessId(), filelog_inc_counter()); | |||
| GetCurrentProcessId(), InterlockedIncrement(&filelog_cnt)); | |||
There was a problem hiding this comment.
This does cause a small change compared to the mutex version: the first file is now numbered 1 rather than 0. If that’s undesirable you can just subtract 1 from the result of the interlocked increment to go back to the previous behaviour. Or maybe it doesn’t matter.
There was a problem hiding this comment.
It does, but indeed it doesn't matter:
- the sequencing is not strict, as some loggers might be instantiated but not produce any file (for instance, for those with a low error setting, when no error occurs);
- the counter is shared for all files, so even those files produced by same process/connection and in the same second, the first index will not be 1 (or previously 0), except potentially for the very first file.
So the start offset change is not that relevant.
droberts195
approved these changes
Oct 24, 2018
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an incremental PR on top of feature/gui_config. The commits on top of it are: https://github.com/bpintea/elasticsearch-sql-odbc/pull/1:
This PR brings the functionality to have one dedicated log file per connection.
There is now one logger type, but two types of instances:
Each instance logs into its dedicated file.
The two types above can coexist. The former is configured through an
environment variable, the later through the DSN parameters (using the DSN GUI editor).
The handle logging macros (INFOH, DBGH etc.) will use the per-connection logger, if the given handle parameter is associated with a connection with dedicated logging enabled. If that's not the case, the "global", per-process logger will be used if that's available (=the environment var is defined).
The non-handle logging macros (INFO, DBG etc.) can still only make use of the per-process logger, since the logger reference is stored in the handles common header, which is not provided with these macros.
This PR also brings the following changes: