Skip to content

go-tarantool/v2/pool.Add: no logs on single connection failure #389

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
Maximilan4 opened this issue Mar 22, 2024 · 2 comments
Open

go-tarantool/v2/pool.Add: no logs on single connection failure #389

Maximilan4 opened this issue Mar 22, 2024 · 2 comments
Labels
2sp feature A new functionality teamE

Comments

@Maximilan4
Copy link

Create pool with empty set of instances:

p, err := pool.ConnectWithOpts(ctx, []pool.Instance{}, connOpts)
...

Later, try to add new instances via add method:

if err = p.Add(ctx, instance); err != nil {
  return fmt.Errorf("conn [%s] err: %w", instance.Name, err)
}

When unable to establish connection with tarantool, i got no err here, no message in logs.
First of all, no any logging at pool/connection_pool.go:285, if canceled variable is false.
Second - no any err handling and logging at pool/connection_pool.go:1419.

Easy proposal: add some basic log.Printf for logging errors in code above.
Hard proposal:

  1. expand basic Logger interface by basic logging methods (Info)
  2. remove method Report from Logger
  3. move method Report from defaultLogger to a specific or common dealer, which always logs specific events with Logger methods
  4. use Logger more often in code

This can allow:

  1. use custom logger inside a connector by writing specific adapter
  2. do not touch default log.SetOutput
  3. manage of verbose level
@oleg-jukovec oleg-jukovec added 1sp teamE feature A new functionality 2sp and removed 1sp labels Mar 23, 2024
@oleg-jukovec
Copy link
Collaborator

oleg-jukovec commented Mar 23, 2024

Thank you for the feedback!

This doesn't look like a priority bug or issue, so I can't promise we'll start working on it soon. But you could create a pull request with Easy proposal: add some basic log.Printf for logging errors in code above. and we will review/merge it.

We have some ideas to improve logging for ConnectionPool so the proposal was added to the backlog.

@Maximilan4
Copy link
Author

Alright, i`ll try to do a request with logs in described parts today. Backlog state looks good.

Maximilan4 pushed a commit to Maximilan4/go-tarantool that referenced this issue Mar 25, 2024
Maximilan4 pushed a commit to Maximilan4/go-tarantool that referenced this issue Mar 25, 2024
Maximilan4 pushed a commit to Maximilan4/go-tarantool that referenced this issue Mar 25, 2024
Add err log to ConnectionPool.Add() in case, when unable to establish
connection and ctx is not canceled; also added logs for error case of
ConnectionPool.tryConnect() calls in ConnectionPool.controller() and ConnectionPool.reconnect()

Part of tarantool#389
oleg-jukovec pushed a commit that referenced this issue Mar 26, 2024
Add err log to ConnectionPool.Add() in case, when unable to establish
connection and ctx is not canceled; also added logs for error case of
ConnectionPool.tryConnect() calls in ConnectionPool.controller() and ConnectionPool.reconnect()

Part of #389
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
2sp feature A new functionality teamE
Projects
None yet
Development

No branches or pull requests

2 participants