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

Otel improvements #1306

Merged
merged 8 commits into from
Jan 30, 2023
Merged

Otel improvements #1306

merged 8 commits into from
Jan 30, 2023

Conversation

markphelps
Copy link
Collaborator

Fixes: FLI-194

Adds some telemetry span attributes for the Evaluate and GetFlag endpoints

CleanShot 2023-01-30 at 13 16 15

CleanShot 2023-01-30 at 13 17 02

@markphelps markphelps requested a review from a team as a code owner January 30, 2023 18:17
- "14250:14250"
- "14268:14268"
- "14269:14269"
- "9411:9411"
Copy link
Contributor

Choose a reason for hiding this comment

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

Jaeger is so greedy with ports

@pull-request-size pull-request-size bot added size/M and removed size/L labels Jan 30, 2023
Comment on lines 6 to 14
AttributeMatch = attribute.Key("flipt.match")
AttributeFlag = attribute.Key("flipt.flag")
AttributeFlagEnabled = attribute.Key("flipt.flag_enabled")
AttributeSegment = attribute.Key("flipt.segment")
AttributeReason = attribute.Key("flipt.reason")
AttributeValue = attribute.Key("flipt.value")
AttributeEntityID = attribute.Key("flipt.entity_id")
AttributeRequestID = attribute.Key("flipt.request_id")
AttributeDurationMS = attribute.Key("flipt.duration_ms")
Copy link
Contributor

Choose a reason for hiding this comment

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

Food for thought: going forward I wonder if we can reconcile these: https://github.com/flipt-io/flipt/blob/main/internal/server/metrics/metrics.go#L54-L60

For now, I dont think it is worth it. Just a shame as there is some overlap.
I wish I had namespaced those metrics.go ones like you have.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah i thought about that, the only difference right now is the namespace I think

Copy link
Contributor

@GeorgeMac GeorgeMac left a comment

Choose a reason for hiding this comment

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

Sweet 💪

@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2023

Codecov Report

Merging #1306 (e9fe1a5) into main (cecd7bc) will increase coverage by 0.17%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #1306      +/-   ##
==========================================
+ Coverage   79.92%   80.09%   +0.17%     
==========================================
  Files          43       43              
  Lines        3272     3300      +28     
==========================================
+ Hits         2615     2643      +28     
  Misses        527      527              
  Partials      130      130              
Impacted Files Coverage Δ
internal/server/evaluator.go 94.38% <100.00%> (+0.33%) ⬆️
internal/server/flag.go 78.31% <100.00%> (+3.31%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@GeorgeMac GeorgeMac left a comment

Choose a reason for hiding this comment

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

One last minute consideration.

internal/server/evaluator.go Outdated Show resolved Hide resolved
@markphelps markphelps merged commit 10219db into main Jan 30, 2023
@markphelps markphelps deleted the otel-improvements branch January 30, 2023 20:29
markphelps added a commit that referenced this pull request Jan 31, 2023
* main:
  chore: bump prom verison (#1305)
  Otel improvements (#1306)
  fix: disable csp headers in non-release mode for ui dev (#1304)
  chore(deps): bump go.opentelemetry.io/otel/exporters/jaeger (#1299)
  chore(deps): bump google.golang.org/grpc from 1.52.0 to 1.52.3 (#1303)
  feat(logging): support custom time, level and message keys (#1295)
  fix: get linter running correctly locally (#1296)
  chore(deps): bump go.opentelemetry.io/otel from 1.11.2 to 1.12.0 (#1301)
  chore(deps): bump golangci/golangci-lint-action from 3.3.1 to 3.4.0 (#1298)
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.

None yet

3 participants