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

Added possibility to customize span operationName #77

Merged
merged 2 commits into from
Jun 19, 2022

Conversation

m1x0n
Copy link
Contributor

@m1x0n m1x0n commented Jun 19, 2022

Extended TracerConfig with OperationNameFunc property which allows to override default operation name during span creation.

If there is no OperationNameFunc provided then fallback will be used which covers most of the use-cases.

Extended TracerConfig with OperationNameFunc which allows
to override default operation name during span creation.

If there is no OperationNameFunc provided then fallback
will be used which covers most of the use-cases.
@codecov
Copy link

codecov bot commented Jun 19, 2022

Codecov Report

Merging #77 (9b786c3) into master (ed09afe) will increase coverage by 0.35%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master      #77      +/-   ##
==========================================
+ Coverage   59.43%   59.79%   +0.35%     
==========================================
  Files           8        8              
  Lines         673      679       +6     
==========================================
+ Hits          400      406       +6     
  Misses        252      252              
  Partials       21       21              
Impacted Files Coverage Δ
jaegertracing/jaegertracing.go 56.12% <88.88%> (+1.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed09afe...9b786c3. Read the comment docs.

Copy link
Contributor

@aldas aldas left a comment

Choose a reason for hiding this comment

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

Other than this one remark everything else seems ok!

@@ -62,6 +62,9 @@ type (
// http body limit size (in bytes)
// NOTE: don't specify values larger than 60000 as jaeger can't handle values in span.LogKV larger than 60000 bytes
LimitSize int

// OperationNameFunc composes operation name based on context. Can be used to override default naming
OperationNameFunc func(ctx echo.Context) string
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename that parameter from ctx to c. Named parameters are handy when your IDE (for example GoLand by Jetbrains) does autocompletion and helps to autofill names. By convention echo.Context is usually named as c.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@m1x0n m1x0n requested a review from aldas June 19, 2022 19:42
Copy link
Contributor

@aldas aldas left a comment

Choose a reason for hiding this comment

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

lgtm

@aldas aldas merged commit d0a6a6e into labstack:master Jun 19, 2022
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.

2 participants