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.

Sign up for GitHub

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

Jump to bottom

Log Task Metadata #2027

Merged
merged 1 commit into from Apr 23, 2019
Merged

Log Task Metadata #2027

merged 1 commit into from Apr 23, 2019

Conversation

pjdufour-truss
Copy link
Contributor

@pjdufour-truss pjdufour-truss commented Apr 22, 2019 •
edited

Description

This PR pulls in task metadata from ECS and adds to the logger, including the current ECS cluster, task definition family, and task definition revision.

Reviewer Notes

None

Setup

None

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
Copy link

codecov bot commented Apr 22, 2019 •
edited

Codecov Report

Merging #2027 into master will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #2027      +/-   ##
=========================================
+ Coverage   61.25%   61.3%   +0.05%     
=========================================
  Files         192     192              
  Lines       12409   12293     -116     
=========================================
- Hits         7601    7536      -65     
+ Misses       3916    3871      -45     
+ Partials      892     886       -6

chrisgilmerproj
chrisgilmerproj reviewed Apr 23, 2019
View reviewed changes
cmd/milmove/main.go
if err != nil {
logger.Error(errors.Wrap(err, "could not read task metadata").Error())
} else {
taskMetadata := &ecs.TaskMetadata{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use an AWS Go SDK type for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It felt like using struct in https://github.com/aws/amazon-ecs-agent/blob/master/misc/taskmetadata-validator/taskmetadata-validator.go was bringing on a big dependency for a little value. I couldn't find anything in the normal GO AWS SDK.

chrisgilmerproj
chrisgilmerproj approved these changes Apr 23, 2019
View reviewed changes
Copy link
Contributor

@chrisgilmerproj chrisgilmerproj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@pjdufour-truss pjdufour-truss merged commit ac29aa5 into master Apr 23, 2019
@pjdufour-truss pjdufour-truss deleted the log_taskdef branch April 23, 2019 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisgilmerproj chrisgilmerproj chrisgilmerproj approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pjdufour-truss @chrisgilmerproj