Log tracing #652
Log tracing #652
Conversation
swalchemist
requested review from
bruce-ricard,
strehle,
tack-sap and
torsten-sap
|
We have created an issue in Pivotal Tracker to manage this: https://www.pivotaltracker.com/story/show/185849184 The labels on this github issue will be updated when the story is started. |
|
Do we want to have a separate UAA repo PR for the source code changes, rather than reviewing them from here through the submodule? I find it a bit awkward. I was the one who bumped the submodule in the uaa-release feature branch, though it was for ease of developer use while working on it, not necessarily for the PR. edit: I think that we should make a separate commit: it's essentially impossible to comment on the UAA changes. |
There is a separate PR for UAA - cloudfoundry/uaa#2446. Committing the submodule change here was probably not helpful, and we shouldn't review specific uaa changes in a uaa-release PR anyway. |
bruce-ricard
force-pushed
the
185000950-trace-logs
branch
from
3c67be4 to
2b780b9
Compare
|
I removed the UAA change from this PR. We now need to merge the UAA one before we merge this one. |
swalchemist
force-pushed
the
185000950-trace-logs
branch
from
5f6b115 to
763d465
Compare
|
I have removed a giant mess of unnecessary commits from this branch. I'm not aware of any actual code change that this causes, but I'm not sure that nothing changed. |
| DescribeTable("UAA log format", func( | ||
| uaaLogFormat string, | ||
| optFiles ...string, | ||
| ) { |
There was a problem hiding this comment.
I'd rather not see this kind of reformatting as part of the PR.
There was a problem hiding this comment.
Hmmm, interesting. For Go, the reformatting is usually automatic and something that's very difficult to suppress. So the code this was changed probably wasn't changed with an IDE.
There was a problem hiding this comment.
If it's coming from the IDE, and they aren't intended line changes, they can can be unselected.
| func getFingerPrint(certdata []byte) ( | ||
| string, | ||
| error, | ||
| ) { |
There was a problem hiding this comment.
The same. I'd rather not see this kind of reformatting as part of the PR.
* The traceId and spanId are populated by the Brave library. Co-authored-by: Bruce Ricard <[email protected]>
* Necessary after the test was fixed so that the regex no longer slurps both [] pairs.
Odd that the log4j pattern must be repeated in so many places.
Trying to fix test failure in CI: "./uaa-release_test.go:48:3: undefined: Expect".
Tallicia
force-pushed
the
185000950-trace-logs
branch
from
7664adf to
78dabb6
Compare
Log tracing - this changes the UAA log format used in a bosh uaa deployment to include a trace ID and span ID, and adjusts the tests to match.
This should be merged after cloudfoundry/uaa#2446 in order for the tests to pass. See that PR for details about the feature.
[#185000950]