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

milmove trace id #2024

Open
wants to merge 1 commit into
base: master
from

Conversation

2 participants
@pjdufour-truss
Copy link
Contributor

commented Apr 22, 2019

Description

This PR creates a unique trace id for every http requests and a related logger that is passed down to each handler. With this approach you can quickly correlate an error message with the specific request and any other info messages that were printed as part of that request.

Additionally, I've add an improvement to the ecs-service-logs program, so you add custom filters as positional arguments, which allows us to filter by milmove-trace-id (or any other property).

Reviewer Notes

  • ecs-service-logs show -c app-experimental -s experimental -n 4 "milmove_trace_id=foo-bar"

Setup

make build_tools

Code Review Verification Steps

  • Code follows the guidelines for Logging
  • The requirements listed in
    Querying the Database Safely
    have been satisfied.
  • Any new migrations/schema changes:
    • Follow our guidelines for zero-downtime deploys (see Zero-Downtime Deploys)
    • Have been communicated to #dp3-engineering
    • Secure migrations have been tested using scripts/run-prod-migrations
  • There are no aXe warnings for UI.
  • This works in Supported Browsers (Chrome, Firefox, IE, Edge).
  • Any new client dependencies (Google Analytics, hosted libraries, CDNs, etc) have been:
  • Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
  • User facing changes have been reviewed by design.
  • Request review from a member of a different team.
  • Have the Pivotal acceptance criteria been met for this change?

References

Screenshots

N.A.

@codecov

This comment has been minimized.

Copy link

commented Apr 22, 2019

Codecov Report

Merging #2024 into master will increase coverage by 0.01%.
The diff coverage is 42.02%.

@@            Coverage Diff             @@
##           master    #2024      +/-   ##
==========================================
+ Coverage   58.98%   58.99%   +0.01%     
==========================================
  Files         238      240       +2     
  Lines       13807    13881      +74     
==========================================
+ Hits         8144     8189      +45     
- Misses       4673     4694      +21     
- Partials      990      998       +8

@pjdufour-truss pjdufour-truss force-pushed the milmove_trace_id branch from 2a5f88b to be9c079 Apr 30, 2019

@pjdufour-truss pjdufour-truss changed the title [WIP] milmove trace id milmove trace id Apr 30, 2019

@@ -431,7 +353,7 @@ func serveFunction(cmd *cobra.Command, args []string) error {
useSecureCookie := !isDevOrTest
// Session management and authentication middleware
noSessionTimeout := v.GetBool(cli.NoSessionTimeoutFlag)
sessionCookieMiddleware := auth.SessionCookieMiddleware(logger, clientAuthSecretKey, noSessionTimeout, appnames, useSecureCookie)
//sessionCookieMiddleware := auth.SessionCookieMiddleware(logger, clientAuthSecretKey, noSessionTimeout, appnames, useSecureCookie)

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj May 1, 2019

Contributor

What's going on with this?

filters[parts[0]] = parts[1]
}
}
}

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj May 1, 2019

Contributor

Is this just saying that any additional args can be passed in without needing a flag? That's pretty neat actually. Could you add a comment though to explain it? Took me a while to track down what was going on by looking at it.

@pjdufour-truss pjdufour-truss force-pushed the milmove_trace_id branch 4 times, most recently from 62d3be8 to 5e47f24 May 1, 2019

@pjdufour-truss pjdufour-truss force-pushed the milmove_trace_id branch 2 times, most recently from bf654a7 to 1fb16a4 May 9, 2019

@pjdufour-truss pjdufour-truss force-pushed the milmove_trace_id branch from 1fb16a4 to 8599ea2 May 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.