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

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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2sp feature A new functionality teamE
Projects
None yet
Development

No branches or pull requests

2 participants