objstore.s3: add trace functionality

[szalai1]

Changes

minio.Client has a TraceOn method which will be
called when one set the traceon: true in the bucket config.

This was a feature request here
#530

Verification

1. Started a minio server locally and created a bucket called test1
2. Created store.yml file (no traceon)

type: S3
config:
  bucket: "test1"
  endpoint: "localhost:9000"
  access_key: "1NRJ8U7TDIKNAJOSG03Q"
  insecure: true
  signature_version2: false
  encrypt_sse: false
  secret_key: "gUFrMkH2PySW5SunGQ3hizixNMwenYu31yZdot+m"
  put_user_metadata: {}
  http_config:
    idle_conn_timeout: 1s
    insecure_skip_verify: true

3. Run ./thanos bucket ls --log.level=debug --objstore.config-file=store.yml
   Output:

 level=info ts=2019-03-18T22:21:58.554021848Z caller=factory.go:39 msg="loading bucket configuration"
level=info ts=2019-03-18T22:21:58.558332917Z caller=main.go:185 msg=exiting

4. Added traceon: true to the configfile and run the same command.
   Output:

level=info ts=2019-03-18T22:21:39.027884607Z caller=factory.go:39 msg="loading bucket configuration"
level=debug ts=2019-03-18T22:21:39.031199041Z caller=stdlib.go:89 s3TraceMsg=---------START-HTTP---------
level=debug ts=2019-03-18T22:21:39.031418548Z caller=stdlib.go:89 s3TraceMsg="GET /test1/?location= HTTP/1.1\r"
level=debug ts=2019-03-18T22:21:39.031470657Z caller=stdlib.go:89 s3TraceMsg="HTTP/1.1 200 OK\r"
level=debug ts=2019-03-18T22:21:39.031484449Z caller=stdlib.go:89 s3TraceMsg=---------END-HTTP---------
level=debug ts=2019-03-18T22:21:39.032384732Z caller=stdlib.go:89 s3TraceMsg=---------START-HTTP---------
level=debug ts=2019-03-18T22:21:39.032596011Z caller=stdlib.go:89 s3TraceMsg="GET /test1/?delimiter=%2F&max-keys=1000&prefix= HTTP/1.1\r"
level=debug ts=2019-03-18T22:21:39.032632794Z caller=stdlib.go:89 s3TraceMsg="HTTP/1.1 200 OK\r"
level=debug ts=2019-03-18T22:21:39.032646396Z caller=stdlib.go:89 s3TraceMsg=---------END-HTTP---------
level=info ts=2019-03-18T22:21:39.032808851Z caller=main.go:185 msg=exiting

5. Set traceon: false. Got the same result as in 3.

[GiedriusS]

This is awesome, thank you very much for the contribution! My only concern: we should probably make the configuration look like this:

trace:
  enable: true

so that in the future we could extend it by another field like only_errors or some other one that we might think of. This is because it might be even more useful to print only non-2xx responses. Perhaps someone will improve minio-go on that side, and then it will be neat to have everything grouped under one key (: What do you think?

[domgreen]

My 2cent - I thinktraceon is okay from reading the docs for traceon it seems that the client either sets it on or off. I'm not sure if we would want to start building custom logic over the client.

I caveat this with I have never used minio so will defer to those who have more experience.

[szalai1]

I am happy to change it to whichever you prefer.
(Designing future proof configuration makes sense if we anticipate changes on minio's side, otherwise, I would not complicate the configuration.)

[GiedriusS]

It's not about anticipating, it's just that printing each and every response will produce lots of spam, and the user would be the most interested in requests which are not successful. Presumably users would only enable this flag if they need to debug a problem with the S3 connectivity. Plus, if it doesn't take a lot of effort to make it very easily extendable, why not? I wanted to see something like this:

trace:
  enable: true

Then in the future if minio-go will support the functionality that I was talking before, we'd add another field which will control what requests and/or responses will be printed. Then, it could look like this:

trace:
  enable: true
  only_errors: true

What's your opinion on this matter?

[GiedriusS]

Awesome work, thank you a lot!

[GiedriusS]

Filled an issue upstream to follow up on this extra functionality: minio/minio-go#1090. 😄

[szalai1]

@GiedriusS funny, I started working on that before you filed the issue. :) Probably I should have filed it too.