otelfiber: add WithCustomMetricAttrbutes option #1159
Conversation
hwo411
requested review from
gaby,
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
WalkthroughThe recent modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Fiber
participant MetricCollector
participant Config
User->>Fiber: Sends request
Fiber->>Config: Retrieves CustomMetricAttributes
Config->>Fiber: Returns custom attributes function
Fiber->>MetricCollector: Collects metrics with custom attributes
MetricCollector-->>Fiber: Metrics recorded
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- otelfiber/config.go (2 hunks)
- otelfiber/fiber.go (1 hunks)
- otelfiber/otelfiber_test/fiber_test.go (1 hunks)
- otelfiber/semconv.go (1 hunks)
Files skipped from review due to trivial changes (1)
- otelfiber/fiber.go
Additional comments not posted (4)
otelfiber/semconv.go (1)
29-31: LGTM! The new conditional block correctly integrates custom metric attributes.The changes are correct and logically sound.
otelfiber/config.go (2)
21-21: LGTM! The new fieldCustomMetricAttributesis correctly added to theconfigstruct.The changes are correct and logically sound.
102-108: LGTM! The new functionWithCustomMetricAttributesis correctly added and integrated.The changes are correct and logically sound.
otelfiber/otelfiber_test/fiber_test.go (1)
450-502: LGTM! The new test functionTestCustomMetricAttributesis correctly added and provides adequate coverage.The changes are correct and logically sound.
|
Great! Let me push the README then |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
otelfiber/README.md (2)
39-39: Correct the verb tense.The verb "returned" should be in the present tense "returns" to fit the context better.
- Define a function to skip this middleware when returned true. + Define a function to skip this middleware when it returns true.Tools
LanguageTool
[uncategorized] ~39-~39: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...a function to skip this middleware when returned true .| nil ...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
45-45: Add missing comma.A comma is missing before "and" in the description.
- Takes a function that will be called on every request and the returned string will become the span Name. + Takes a function that will be called on every request, and the returned string will become the span Name.Tools
LanguageTool
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...a function that will be called on every request and the returned string will become the...(AI_HYDRA_LEO_MISSING_COMMA)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- otelfiber/README.md (1 hunks)
Additional context used
LanguageTool
otelfiber/README.md
[uncategorized] ~39-~39: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...a function to skip this middleware when returned true .| nil ...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...a function that will be called on every request and the returned string will become the...(AI_HYDRA_LEO_MISSING_COMMA)
Additional comments not posted (1)
otelfiber/README.md (1)
47-47: LGTM! Verify usage examples.The addition of the
WithCustomMetricAttributesparameter is consistent and well-documented.Ensure that the usage examples in the documentation are updated to reflect this new parameter.
Add
WithCustomMetricAttributeswhich is analogous toWithCustomAttributesfor span.Alternatively, we can apply
WithCustmAttributesto metrics, but it doesn't give enough control over attributesFixes #1156
(If the solution is good, I'll update the README then)
Summary by CodeRabbit
New Features
Bug Fixes
Tests