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

Add access logging walkthrough #509

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Add access logging walkthrough #509

wants to merge 16 commits into from

Conversation

EdwardXF
Copy link
Contributor

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.

@EdwardXF EdwardXF requested a review from a team as a code owner August 31, 2022 22:17
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
Copy link
Contributor

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.

Copy link
Contributor Author

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

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

Copy link
Contributor Author

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

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.

Copy link
Contributor Author

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

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

Copy link
Contributor Author

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

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

Copy link
Contributor Author

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

Copy link
Contributor Author

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

@joesbigidea
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants