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

refactor!: prometheus attributes format to otel attributes #4122

Closed
wants to merge 9 commits into from
Closed

refactor!: prometheus attributes format to otel attributes #4122

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d441999
refactor: prometheus attributes format to otel attributes
varshith257 Oct 26, 2024
657e109
Update OpenTelemetryMetrics.scala
varshith257 Oct 30, 2024
a6c7d6c
refactor!: prometheus and OTEL defaul Metric formats
varshith257 Oct 30, 2024
ce71870
feat!: add review comments
varshith257 Nov 3, 2024
1d8c612
feat: add req additional attributes
varshith257 Nov 8, 2024
c8c1970
feat: move OTEL metrics related code to OTEL Module
varshith257 Nov 8, 2024
ba7b18f
cleanup
varshith257 Nov 8, 2024
6733960
Try to return an Option[String] with review suggestions
varshith257 Nov 16, 2024
c654d5a
Fix compilation error
varshith257 Nov 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 38 additions & 10 deletions ...metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ import java.time.{Duration, Instant}
case class OpenTelemetryMetrics[F[_]](meter: Meter, metrics: List[Metric[F, _]]) {

/** Registers a `request_active{path, method}` up-down-counter (assuming default labels). */
def addRequestsActive(labels: MetricLabels = MetricLabels.Default): OpenTelemetryMetrics[F] =
def addRequestsActive(labels: MetricLabels = OpenTelemetryAttributes): OpenTelemetryMetrics[F] =
copy(metrics = metrics :+ requestActive(meter, labels))

/** Registers a `request_total{path, method, status}` counter (assuming default labels). */
def addRequestsTotal(labels: MetricLabels = MetricLabels.Default): OpenTelemetryMetrics[F] =
def addRequestsTotal(labels: MetricLabels = OpenTelemetryAttributes): OpenTelemetryMetrics[F] =
copy(metrics = metrics :+ requestTotal(meter, labels))

/** Registers a `request_duration_seconds{path, method, status, phase}` histogram (assuming default labels). */
def addRequestsDuration(labels: MetricLabels = MetricLabels.Default): OpenTelemetryMetrics[F] =
def addRequestsDuration(labels: MetricLabels = OpenTelemetryAttributes): OpenTelemetryMetrics[F] =
copy(metrics = metrics :+ requestDuration(meter, labels))

/** Registers a custom metric. */
Expand All @@ -36,6 +36,33 @@ case class OpenTelemetryMetrics[F[_]](meter: Meter, metrics: List[Metric[F, _]])

object OpenTelemetryMetrics {

/** Default labels for OpenTelemetry-compliant metrics, as recommended here:
* https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#http-server
*
* - `http.request.method` - HTTP request method (e.g., GET, POST).
* - `path` - The request path or route template.
* - `http.response.status_code` - HTTP response status code (200, 404, etc.).
*/
lazy val OpenTelemetryAttributes: MetricLabels = MetricLabels(
forRequest = List(
"http.request.method" -> { case (_, req) => req.method.method },
"url.scheme" -> { case (_, req) => req.uri.scheme.getOrElse("unknown") },
"path" -> { case (ep, _) => ep.showPathTemplate(showQueryParam = None) }
),
forResponse = List(
"http.response.status_code" -> {
case Right(r) => r.code.code.toString
// Default to 500 for exceptions
case Left(_) => "500"
},
"error.type" -> {
case Left(ex) => ex.getClass.getSimpleName
case Right(_) => ""
Copy link
Member

Choose a reason for hiding this comment

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

I think if the response is a Right we shouldn't add any attributes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it will helps.Will make changes for it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adamw The scala compiler is hungry to handle Right case?
WDYT the better way what you are thinking of?

Copy link
Contributor Author

@varshith257 varshith257 Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@adamw

Copy link
Member

Choose a reason for hiding this comment

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

ah, well I guess this would have to return an Option[String], not a String, and then you would report only the defined labels

Copy link
Contributor Author

@varshith257 varshith257 Nov 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@adamw It's getting a bit tricky making "error.type" to Option[String]. In that case, if we make forResponse then h http.response.status_code could go None for bad req?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this comment 3 days before but don't know why not posted here lol

Copy link
Member

Choose a reason for hiding this comment

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

yes, if an attribute is None we don't include it, if it's a Some we do

varshith257 marked this conversation as resolved.
Show resolved Hide resolved

}
)
varshith257 marked this conversation as resolved.
Show resolved Hide resolved
)

def apply[F[_]](meter: Meter): OpenTelemetryMetrics[F] = apply(meter, Nil)
def apply[F[_]](otel: OpenTelemetry): OpenTelemetryMetrics[F] = apply(defaultMeter(otel), Nil)
def apply[F[_]](otel: OpenTelemetry, metrics: List[Metric[F, _]]): OpenTelemetryMetrics[F] = apply(defaultMeter(otel), metrics)
Expand All @@ -50,7 +77,7 @@ object OpenTelemetryMetrics {
* measured separately up to the point where the headers are determined, and then once again when the whole response body is complete.
*/
def default[F[_]](otel: OpenTelemetry): OpenTelemetryMetrics[F] =
default(defaultMeter(otel), MetricLabels.Default)
default(defaultMeter(otel), OpenTelemetryAttributes)

/** Registers default metrics (see other variants) using custom labels. */
def default[F[_]](otel: OpenTelemetry, labels: MetricLabels): OpenTelemetryMetrics[F] = default(defaultMeter(otel), labels)
Expand All @@ -64,10 +91,10 @@ object OpenTelemetryMetrics {
* Status is by default the status code class (1xx, 2xx, etc.), and phase can be either `headers` or `body` - request duration is
* measured separately up to the point where the headers are determined, and then once again when the whole response body is complete.
*/
def default[F[_]](meter: Meter): OpenTelemetryMetrics[F] = default(meter, MetricLabels.Default)
def default[F[_]](meter: Meter): OpenTelemetryMetrics[F] = default(meter, OpenTelemetryAttributes)

/** Registers default metrics (see other variants) using custom labels. */
def default[F[_]](meter: Meter, labels: MetricLabels = MetricLabels.Default): OpenTelemetryMetrics[F] =
def default[F[_]](meter: Meter, labels: MetricLabels = OpenTelemetryAttributes): OpenTelemetryMetrics[F] =
OpenTelemetryMetrics(
meter,
List[Metric[F, _]](
Expand All @@ -80,7 +107,7 @@ object OpenTelemetryMetrics {
def requestActive[F[_]](meter: Meter, labels: MetricLabels): Metric[F, LongUpDownCounter] =
Metric[F, LongUpDownCounter](
meter
.upDownCounterBuilder("request_active")
.upDownCounterBuilder("http.server.active_requests")
varshith257 marked this conversation as resolved.
Show resolved Hide resolved
.setDescription("Active HTTP requests")
.setUnit("1")
.build(),
Expand All @@ -97,7 +124,7 @@ object OpenTelemetryMetrics {
def requestTotal[F[_]](meter: Meter, labels: MetricLabels): Metric[F, LongCounter] =
Metric[F, LongCounter](
meter
.counterBuilder("request_total")
.counterBuilder("http.server.request.total")
.setDescription("Total HTTP requests")
.setUnit("1")
.build(),
Expand All @@ -108,6 +135,7 @@ object OpenTelemetryMetrics {
m.eval {
val otLabels =
merge(asOpenTelemetryAttributes(labels, ep, req), asOpenTelemetryAttributes(labels, Right(res), None))

counter.add(1, otLabels)
}
}
Expand All @@ -125,9 +153,9 @@ object OpenTelemetryMetrics {
def requestDuration[F[_]](meter: Meter, labels: MetricLabels): Metric[F, DoubleHistogram] =
Metric[F, DoubleHistogram](
meter
.histogramBuilder("request_duration")
.histogramBuilder("http.server.request.duration")
.setDescription("Duration of HTTP requests")
.setUnit("ms")
.setUnit("s")
.build(),
onRequest = (req, recorder, m) =>
m.eval {
Expand Down
66 changes: 48 additions & 18 deletions ...ics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import scala.concurrent.Future

class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers {

"default metrics" should "collect requests active" in {
"default metrics" should "collect http.server.active_requests" in {
// given
val reader = InMemoryMetricReader.create()
val provider = SdkMeterProvider.builder().registerMetricReader(reader).build()
Expand All @@ -51,17 +51,31 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers {

// then
val point = longSumData(reader).head
point.getAttributes shouldBe Attributes.of(AttributeKey.stringKey("method"), "GET", AttributeKey.stringKey("path"), "/person")
point.getAttributes shouldBe Attributes.of(
AttributeKey.stringKey("http.request.method"),
"GET",
AttributeKey.stringKey("path"),
"/person",
AttributeKey.stringKey("url.scheme"),
"http"
)
point.getValue shouldBe 1

ScalaFutures.whenReady(response, Timeout(Span(3, Seconds))) { _ =>
val point = longSumData(reader).head
point.getAttributes shouldBe Attributes.of(AttributeKey.stringKey("method"), "GET", AttributeKey.stringKey("path"), "/person")
point.getAttributes shouldBe Attributes.of(
AttributeKey.stringKey("http.request.method"),
"GET",
AttributeKey.stringKey("path"),
"/person",
AttributeKey.stringKey("url.scheme"),
"http"
)
point.getValue shouldBe 0
}
}

"default metrics" should "collect requests total" in {
"default metrics" should "collect http.server.request.total" in {
// given
val reader = InMemoryMetricReader.create()
val provider = SdkMeterProvider.builder().registerMetricReader(reader).build()
Expand All @@ -87,29 +101,37 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers {
.count {
case dp
if dp.getAttributes == Attributes.of(
AttributeKey.stringKey("method"),
AttributeKey.stringKey("http.request.method"),
"GET",
AttributeKey.stringKey("path"),
"/person",
AttributeKey.stringKey("status"),
"2xx"
AttributeKey.stringKey("url.scheme"),
"http",
AttributeKey.stringKey("http.response.status_code"),
"200",
AttributeKey.stringKey("error.type"),
""
) && dp.getValue == 2 =>
true
case dp
if dp.getAttributes == Attributes.of(
AttributeKey.stringKey("method"),
AttributeKey.stringKey("http.request.method"),
"GET",
AttributeKey.stringKey("path"),
"/person",
AttributeKey.stringKey("status"),
"4xx"
AttributeKey.stringKey("url.scheme"),
"http",
AttributeKey.stringKey("http.response.status_code"),
"400",
AttributeKey.stringKey("error.type"),
""
) && dp.getValue == 2 =>
true
case _ => false
} shouldBe 2
}

"default metrics" should "collect requests duration" in {
"default metrics" should "collect http.server.request.duration" in {
// given
val reader = InMemoryMetricReader.create()
val provider = SdkMeterProvider.builder().registerMetricReader(reader).build()
Expand Down Expand Up @@ -140,14 +162,18 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers {
val point = reader.collectAllMetrics().asScala.head.getHistogramData.getPoints.asScala
point.map(_.getAttributes) should contain(
Attributes.of(
AttributeKey.stringKey("method"),
AttributeKey.stringKey("http.request.method"),
"GET",
AttributeKey.stringKey("path"),
"/person",
AttributeKey.stringKey("status"),
"2xx",
AttributeKey.stringKey("http.response.status_code"),
"200",
AttributeKey.stringKey("phase"),
"body"
"body",
AttributeKey.stringKey("url.scheme"),
"http",
AttributeKey.stringKey("error.type"),
""
)
)
}
Expand Down Expand Up @@ -197,12 +223,16 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers {
// then
val point = longSumData(reader).head
point.getAttributes shouldBe Attributes.of(
AttributeKey.stringKey("method"),
AttributeKey.stringKey("http.request.method"),
"GET",
AttributeKey.stringKey("path"),
"/person",
AttributeKey.stringKey("status"),
"5xx"
AttributeKey.stringKey("url.scheme"),
"http",
AttributeKey.stringKey("http.response.status_code"),
"500",
AttributeKey.stringKey("error.type"),
""
)
point.getValue shouldBe 1
}
Expand Down