- Notifications
You must be signed in to change notification settings - Fork 15
Conversation
| The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| This is an amazing amount of data to get documented! 🎉 Thank you for all the time this must have taken. |
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.
In total, all of this new verbosity is great; thank you Ryan for the time put into consolidating all this information. Here are my edits/suggestions:
| Spans conform to the following rules: | ||
| | ||
| * Each span has a unique name in plain English, explaining what it represents. | ||
| Due to this descriptive nature of spans, individual trace spans are not |
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.
Providing a list of all available trace would still provide a benefit, as it would allow people to be able to verify how many there are/they should expect, and whether they are successfully ingesting all the ones that are associated to the particular operations they've executed.
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 is the type of documentation that would go stale very easily, which is why I opted to not include it. I think the maintenance burden ends up outweighing the utility here. It's similar to not documenting every log line - I believe it is overly verbose, and the user should be able to get context on any given trace frame from looking at the frame itself. Metrics are a bit different, as users are likely to build monitors around them and depend on them to some extent, and there is not a good cross-platform way of ensuring they provide accurate and understandable context without additional documentation.
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.
That's fair, granted i do want to note that this will make it difficult for the support team to provide guidance to customers when they ask questions about what traces we emit, and how many. this will essentially leave the support engineer to have to manually uncover this information through the source code, or through internal documentation that is hopefully kept up to date. Given the amount of effort that's been put into this new iteration of documentation, and your reasonings for not including it, i'm okay with foregoing this requirement of the Feature Request, and awaiting feedback from Home Depot, and other customers, as to whether this is desired.
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.
Comments about the example commands
| Thanks so much for the thorough read, @zisom-hc ! I applied your suggestions. |
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.
new edits LGTM, 🚢
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 added a couple of tiny spelling nits.
This context/documentation is extremely helpful for the tfc-agent telemetry project. Thank you!
Co-authored-by: Natalie Todd <natalie.todd@hashicorp.com>
Co-authored-by: Natalie Todd <natalie.todd@hashicorp.com>
Co-authored-by: Natalie Todd <natalie.todd@hashicorp.com>
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.
Thank you!
Co-authored-by: Judith Malnick <judith@hashicorp.com>
Co-authored-by: Judith Malnick <judith@hashicorp.com>
Co-authored-by: Judith Malnick <judith@hashicorp.com>
Co-authored-by: Judith Malnick <judith@hashicorp.com>
What
Adds details for telemetry data, including logs, tracing, and metrics. This change attempts to give good coverage of current metrics, without including any details which are subject to change, such as the tags and labels applied to each metric. This is intended to provide a basic inventory of metrics and high-level descriptions of what each measurement means.
Why
Multiple users have asked for this, and this follows the same general pattern for documenting metrics as taken by other products such as Nomad or Consul.
Screenshots
Merge Checklist
If items do not apply to your changes, add (N/A) and mark them as complete.
Pull Request
Content
terraform-websiterepositoryredirects.next.jsfile.Reviews