Skip to content

Commit

Permalink
Merge pull request #78 from NthPortal/boolean-blindness/PR
Browse files Browse the repository at this point in the history
Fix danger of boolean blindness
  • Loading branch information
NthPortal committed May 6, 2024
2 parents 5f0aca4 + b83cb9d commit 678cc98
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 10 deletions.
25 changes: 25 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,31 @@ jobs:
modules-ignore: examples_2.13 examples_3 site_2.13 site_3 sbt-http4s-org-scalafix-internal_2.13 sbt-http4s-org-scalafix-internal_3 http4s-otel4s-middlewarejvm_2.13 http4s-otel4s-middlewarejvm_3 http4s-otel4s-middlewarejs_2.13 http4s-otel4s-middlewarejs_3 http4s-otel4s-middlewarenative_2.13 http4s-otel4s-middlewarenative_3 core-jvm-tests_2.13 core-jvm-tests_3
configs-ignore: test scala-tool scala-doc-tool test-internal

validate-steward:
name: Validate Steward Config
strategy:
matrix:
os: [ubuntu-latest]
java: [temurin@11]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout current branch (fast)
uses: actions/checkout@v4

- name: Setup Java (temurin@11)
id: setup-java-temurin-11
if: matrix.java == 'temurin@11'
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: 11

- uses: coursier/setup-action@v1
with:
apps: scala-steward

- run: scala-steward validate-repo-config .scala-steward.conf

site:
name: Generate Site
strategy:
Expand Down
1 change: 1 addition & 0 deletions .scala-steward.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
updates.ignore = [ { groupId = "org.slf4j", artifactId = "slf4j-simple" } ]
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ object ServerMiddleware {
Defaults.additionalRequestAttributes,
Defaults.additionalResponseAttributes,
Defaults.urlRedactor,
Defaults.doNotTrace,
Defaults.shouldTrace,
)

/** The default configuration values for a server middleware builder. */
Expand All @@ -68,7 +68,8 @@ object ServerMiddleware {
def additionalResponseAttributes[F[_]]: Response[F] => immutable.Iterable[Attribute[_]] =
(_: Response[F]) => Nil
val urlRedactor: UriRedactor = UriRedactor.OnlyRedactUserInfo
def doNotTrace: RequestPrelude => Boolean = (_: RequestPrelude) => false
val shouldTrace: RequestPrelude => ShouldTrace =
(_: RequestPrelude) => ShouldTrace.Trace
}

/** A builder for server middlewares. */
Expand All @@ -80,7 +81,7 @@ object ServerMiddleware {
additionalRequestAttributes: Request[F] => immutable.Iterable[Attribute[_]],
additionalResponseAttributes: Response[F] => immutable.Iterable[Attribute[_]],
urlRedactor: UriRedactor,
doNotTrace: RequestPrelude => Boolean,
shouldTrace: RequestPrelude => ShouldTrace,
) {
private def copy(
allowedRequestHeaders: Set[CIString] = this.allowedRequestHeaders,
Expand All @@ -92,7 +93,7 @@ object ServerMiddleware {
additionalResponseAttributes: Response[F] => immutable.Iterable[Attribute[_]] =
this.additionalResponseAttributes,
urlRedactor: UriRedactor = this.urlRedactor,
doNotTrace: RequestPrelude => Boolean = this.doNotTrace,
shouldTrace: RequestPrelude => ShouldTrace = this.shouldTrace,
): ServerMiddlewareBuilder[F] =
new ServerMiddlewareBuilder[F](
allowedRequestHeaders,
Expand All @@ -102,7 +103,7 @@ object ServerMiddleware {
additionalRequestAttributes,
additionalResponseAttributes,
urlRedactor,
doNotTrace,
shouldTrace,
)

/** Sets which request headers are allowed to made into `Attribute`s. */
Expand Down Expand Up @@ -146,9 +147,9 @@ object ServerMiddleware {
def withUrlRedactor(urlRedactor: UriRedactor): ServerMiddlewareBuilder[F] =
copy(urlRedactor = urlRedactor)

/** Sets how to determine when not to trace a request and its response. */
def withDoNotTrace(doNotTrace: RequestPrelude => Boolean): ServerMiddlewareBuilder[F] =
copy(doNotTrace = doNotTrace)
/** Sets how to determine when to trace a request and its response. */
def withShouldTrace(shouldTrace: RequestPrelude => ShouldTrace): ServerMiddlewareBuilder[F] =
copy(shouldTrace = shouldTrace)

/** Returns a middleware in a way that abstracts over
* [[org.http4s.HttpApp `HttpApp`]] and
Expand All @@ -162,8 +163,12 @@ object ServerMiddleware {
f: Http[G, F]
)(implicit kt: KindTransformer[F, G]): Http[G, F] =
Kleisli { (req: Request[F]) =>
if (doNotTrace(req.requestPrelude) || !Tracer[F].meta.isEnabled) f(req)
else {
if (
shouldTrace(req.requestPrelude) == ShouldTrace.DoNotTrace ||
!Tracer[F].meta.isEnabled
) {
f(req)
} else {
val init =
request(
req,
Expand Down
29 changes: 29 additions & 0 deletions core/src/main/scala/org/http4s/otel4s/middleware/ShouldTrace.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright 2023 http4s.org
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.http4s.otel4s.middleware

/** Whether or not to trace something. */
sealed trait ShouldTrace

object ShouldTrace {

/** Trace the thing. */
case object Trace extends ShouldTrace

/** Do not trace the thing. */
case object DoNotTrace extends ShouldTrace
}

0 comments on commit 678cc98

Please sign in to comment.