-
Notifications
You must be signed in to change notification settings - Fork 394
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
Add access logging walkthrough #509
base: main
Are you sure you want to change the base?
Conversation
1. check meshes is deployed | ||
|
||
```bash | ||
aws --endpoint-url https://frontend.$AWS_DEFAULT_REGION.prod.lattice.aws.a2z.com --region $AWS_DEFAULT_REGION appmesh list-meshes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be good to drop --endpoint-url on all of these, it's a little odd for customers to be specifying now that the feature is available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it
To test: | ||
|
||
```bash | ||
colorapp=$(aws cloudformation describe-stacks --region=$AWS_DEFAULT_REGION --stack-name=$ENVIRONMENT_NAME-ecs-colorapp --query="Stacks[0].Outputs[?OutputKey=='ColorAppEndpoint'].OutputValue" --output=text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I don't think there's any need for --region=$AWS_DEFAULT_REGION on aws cli commands. I think that var takes effect automatically. https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-envvars.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, will delete that
``` | ||
[2022-08-04T23:00:29.383Z] "GET /ping HTTP/1.1" 200 - 0 0 1 0 "-" "Envoy/HC" "05ba7454-fce2-9360-92e3-a4da997d4f44" "colorteller-blue.logging.local:9080" "127.0.0.1:9080" | ||
``` | ||
6. change between different formats or change the pattern to find bugs! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably doesn't belong in customer facing stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah
@@ -0,0 +1,174 @@ | |||
## Access Log Format Feature Background | |||
Today App Mesh supports configuring access log file path (https://docs.aws.amazon.com/app-mesh/latest/userguide/envoy-logs.html) for virtual nodes (https://docs.aws.amazon.com/app-mesh/latest/userguide/virtual_nodes.html) and virtual gateways (https://docs.aws.amazon.com/app-mesh/latest/userguide/virtual_gateways.html). We apply the same configuration for all the listener filter chains in Envoy. If not specified, Envoy will output logs to /dev/stdout by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think maybe this background should be rewritten, it seems targeted at folks testing an unreleased feature, talking about the current state of the system an adding a new feature, while for the audience of this example the logging format is an existing feature they want to learn about
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do
@@ -0,0 +1,231 @@ | |||
# Using AWS App Mesh with Fargate | |||
|
|||
## Overview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This readme is really odd in the context of the access log example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will reconstruct this example, not using the bug bash, it will be easier for customer to use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the console changes are out, customers should be using that as the major way for accessing this feature
There are some files and diagrams that don't seem really relevant to the access logging config in here. I feel like it'll be easier for our customers to focus on the access log feature and understand it if the extra stuff is stripped out. |
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.