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

tracing: Support placeholders in span name #5329

Merged
merged 1 commit into from
Jan 25, 2023
Merged

tracing: Support placeholders in span name #5329

merged 1 commit into from
Jan 25, 2023

Conversation

davidfrickert
Copy link
Contributor

@davidfrickert davidfrickert commented Jan 23, 2023 •
edited
Loading

Fixes #5171

Notes:

  • I'm a noob in Go - this was my first tackle at using the language.
  • I fixed the tests that started breaking, but did not add any new tests. I have no idea how to check the output span after it has been formatted in the tests.
  • I built caddy and used it in my Server and checked that the output spans are correct - as in the placeholders are replaced.

@CLAassistant
Copy link

CLAassistant commented Jan 23, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@davidfrickert davidfrickert changed the title [Tracing] Replace placeholders in span name [Tracing] Support placeholders in span name Jan 23, 2023
@francislavoie francislavoie added the feature ⚙️ New feature or request label Jan 23, 2023
@francislavoie francislavoie added this to the v2.6.3 milestone Jan 23, 2023
@francislavoie francislavoie changed the title [Tracing] Support placeholders in span name tracing: Support placeholders in span name Jan 25, 2023
francislavoie
francislavoie approved these changes Jan 25, 2023
View reviewed changes
Copy link
Member

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thanks for contributing!

@francislavoie francislavoie merged commit 9623102 into caddyserver:master Jan 25, 2023
@cdaguerre
Copy link

thanks @davidfrickert !!
Is there an easy way to figure out which placeholders are actually supported?
To my understanding, these should be supported, but not the response, correct?

@francislavoie
Copy link
Member

Yeah, basically any request that reads from the request should work. Those you linked are the Caddyfile shortcuts, but many more are in https://caddyserver.com/docs/json/apps/http/#docs. Depends what you want to do with it.

@davidfrickert davidfrickert deleted the issue-5171 branch January 25, 2023 10:10
@davidfrickert
Copy link
Contributor Author

No problem @francislavoie @cdaguerre.
My pleasure to contribute to such an awesome project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mohammed90 mohammed90 mohammed90 left review comments

@francislavoie francislavoie francislavoie approved these changes

Assignees
No one assigned
Labels
feature ⚙️ New feature or request
Projects
None yet
Milestone
v2.6.3
Development

Successfully merging this pull request may close these issues.

[opentelemetry] add ability to contextualize span name
5 participants
@davidfrickert @CLAassistant @cdaguerre @francislavoie @mohammed90