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

Deprecate old configuration parameters in MongoDB destination #893

Closed
ihrwein opened this issue Feb 2, 2016 · 8 comments
Closed

Deprecate old configuration parameters in MongoDB destination #893

ihrwein opened this issue Feb 2, 2016 · 8 comments
Assignees
Labels

Comments

@ihrwein
Copy link
Contributor

ihrwein commented Feb 2, 2016

Migrating some comments from #891 (comment):

me:

Configuration changes: `servers`, `host`, `port`, `path`, `database`, `username`, `password`, `safe-mode` are removed.  `uri` is added, which accepts a mongodb uri as its parameter (https://docs.mongodb.org/manual/reference/connection-string/).

These changes are effective on the master branch and will be published first in syslog-ng 3.8. @fekete-robert may be also interested in this.

@bazsi:

Telling the user to migrate to the new options is fine and is the policy we applied in the past. We can even remove options if a long enough grace period is available.

The fact that 3.8 is a major release is not an excuse to break user configurations, our intent is to make upgrades (even accross major versions) as seamless as possible. 

Breaking configurations this way would upset users of that feature and rightly so.

I would consider this a critical bug.
@ihrwein ihrwein added the bug label Feb 2, 2016
@bkil-syslogng
Copy link
Contributor

It's a good idea to open a new issue for this question. However I would probably adjust the naming and description a bit.

Which route should we take?

  • rename a URI based client to a new module
  • hack both mongo-c-driver and libmongo-client into the same module
  • implement some of the previous config keywords with common semantics using the mongo-c-driver URI connection

For how long should we keep compatibility?

How will all of this affect our tests?

Who will provide real world use cases to test on to ensure that we are indeed retaining compatibility for these cases? Our trivial synthetics don't exercise the capabilities that much (or at all) from an enterprise perspective. Unfortunately, I have a feeling that the bugs that will be reported in the future will lie mostly within such corner cases.

@ihrwein
Copy link
Contributor Author

ihrwein commented Feb 3, 2016

@bkil-syslogng Feel free to rename the title and change the description as you wish.

@bazsi
Copy link
Collaborator

bazsi commented Feb 3, 2016

I would pick the 3rd option, translate old parameters into the URI, while
reporting config warnings that using the URI is the preferred way of using
the mongodb driver.

Definitely not duplicating the entire module, nor using two transport
mechanisms. I don't know enough about the new scheme, but I sincerely hope
that all the previous parameters have some kind of equivalent in the URI
scheme.

Bazsi

Bazsi

On Wed, Feb 3, 2016 at 2:08 PM, Tamas Nagy notifications@github.com wrote:

It's a good idea to open a new issue for this question. However I would
probably adjust the naming and description a bit.

Which route should we take?

  • rename a URI based client to a new module
  • hack both mongo-c-driver and libmongo-client into the same module
  • implement some of the previous config keywords with common semantics
    using the mongo-c-driver URI connection

For how long should we keep compatibility?

How will all of this affect our tests?

Who will provide real world use cases to test on to ensure that we are
indeed retaining compatibility for these cases? Our trivial synthetics
don't exercise the capabilities that much (or at all) from an enterprise
perspective. Unfortunately, I have a feeling that the bugs that will be
reported in the future will lie mostly within such corner cases.


Reply to this email directly or view it on GitHub
#893 (comment).

@bkil-syslogng
Copy link
Contributor

It was many months before I was into this topic, however I'm sure multiple minor options are missing semantically equivalent translations. I mean, you could translate them, but it might behave in a different way (replica set formation, synchronization, in case of errors, or in general, etc).

I could volunteer to (re-)apply and adjust the former, basic, non-semantically equivalent syntactical sugar. It would make some of the previous sandbox and blog posted examples pass.

However, afterwards, I decline to maintain this implementation, so if anythings breaks, or any support or investigation comes in along this route, you will need to find another developer to debug, fix or extend this code. If this is okay with you, I will assign this task to myself and we will further schedule when I should work on implementing this, depending on our priorities.

Just so you know, when we were discussing refactoring MongoDB support, we did even (jokingly) consider removing it altogether because a lack of usage and interest.

@bazsi
Copy link
Collaborator

bazsi commented Feb 3, 2016

Hi,

First of all, thanks for your consideration. Some more answers inline.

On Wed, Feb 3, 2016 at 2:40 PM, Tamas Nagy notifications@github.com wrote:

It was many months before I was into this topic, however I'm sure multiple
minor options are missing semantically equivalent translations. I mean,
you could translate them, but it might behave in a different way (replica
set formation, synchronization, in case of errors, or in general, etc).

I think I probably miss a few points, so it would definitely make sense for
me to understand the difficulty a bit more.

I could volunteer to (re-)apply and adjust the former, basic,
non-semantically equivalent syntactical sugar. It would make some of the
previous sandbox and blog posted examples pass.

and a unit test of the same?

However, afterwards, I decline to maintain this implementation, so if
anythings breaks, or any support or investigation comes in along this
route, you will need to find another developer to debug, fix or extend this
code. If this is okay with you, I will assign this task to myself and we
will further schedule when I should work on implementing this, depending on
our priorities.

Just so you know, when we were discussing refactoring MongoDB support, we
did even (jokingly) consider removing it altogether because a lack of usage
and interest.

How did you measure that? There even was at one point where we had
syslog-ng-web, a web application that relied on syslog-ng + mongo as a
backend. Interest might have peaked already, and this might be past its
prime time, but then we should validate that (by contacting the community,
generally making some buzz about it), and then perhaps remove it completely.

Bazsi

@bkil-syslogng bkil-syslogng self-assigned this Feb 4, 2016
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue Mar 7, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue Mar 8, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
@bkil-syslogng
Copy link
Contributor

A question for @bazsi, @pzoleex and others interested in MongoDB:

Should the semantics of safe_mode(no) include turning off client side input document validation via MONGOC_INSERT_NO_VALIDATE and leave it to the server (if it supports that at all)?

http://api.mongodb.org/c/current/mongoc_insert_flags_t.html
https://github.com/mongodb/mongo-c-driver/blob/master/src/mongoc/mongoc-collection.c#L1261

With the original connector, this mostly controlled synchronicity and write concern.

@lbudai
Copy link
Collaborator

lbudai commented Mar 11, 2016

@bkil-syslogng : safe_mode(yes) means (in libmongoclient) that after each insert we request a status from the server via the getlasterror command. Maybe we can replace this by using write concerns...
and client side input validation is a different thing.

Based on this we should move to use the write concerns: https://docs.mongodb.org/manual/reference/command/getLastError/

@lbudai
Copy link
Collaborator

lbudai commented Mar 17, 2016

@bkil-syslogng : any update?

bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue Mar 17, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue Mar 17, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue Apr 11, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue Apr 13, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue Apr 13, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

A large part of the code comes from the previous state before being
removed:

6636898
"afmongodb: added mongo uri option, removed all redundant ones"

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue Apr 19, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

A large part of the code comes from the previous state before being
removed:

6636898
"afmongodb: added mongo uri option, removed all redundant ones"

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue Apr 21, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

A large part of the code comes from the previous state before being
removed:

6636898
"afmongodb: added mongo uri option, removed all redundant ones"

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue May 13, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

A large part of the code comes from the previous state before being
removed:

6636898
"afmongodb: added mongo uri option, removed all redundant ones"

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
(cherry picked from commit b5cd1e3)
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue May 20, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

A large part of the code comes from the previous state before being
removed:

6636898
"afmongodb: added mongo uri option, removed all redundant ones"

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue May 20, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

A large part of the code comes from the previous state before being
removed:

6636898
"afmongodb: added mongo uri option, removed all redundant ones"

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue May 20, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

A large part of the code comes from the previous state before being
removed:

6636898
"afmongodb: added mongo uri option, removed all redundant ones"

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
bkil-syslogng added a commit to bkil-syslogng/syslog-ng that referenced this issue May 24, 2016
This re-adds additional support for previous syntax used by libmongo-client
before we started using mongo-c-driver and its URI syntax exclusively.

I.e., so additionally to uri() and collection(), now the following
keywords should work again:

servers()
safe_mode()
path()
password()
username()
database()

Note that these are plainly translated to a connection URI without
much sanity checking or preserving their former semantic meaning.
So various aspects of the MongoDB connection like health checks, retries,
error reporting and synchronicity will still follow the slightly altered
semantics of mongo-c-driver.

A large part of the code comes from the previous state before being
removed:

6636898
"afmongodb: added mongo uri option, removed all redundant ones"

In the future, theoretically it is enough to revert this single commit
to remove support.
(This is one reason I did not opt for OOP & inheritance to implement this)

Fixes syslog-ng#893

Signed-off-by: bkil-syslogng <tamas.nagy@balabit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants