-
Notifications
You must be signed in to change notification settings - Fork 40
Split project into modules #39
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ import org.typelevel.otel4s.metrics.MeterProvider | |
|
||
trait Otel4s[F[_]] { | ||
|
||
/** A registry for creating named [[org.typelevel.otel4s.metrics.Meter]]. | ||
/** A registry for creating named meters. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can use a pipe operator if you want it to say [[org.typelevel.otel4s.metrics.Meter|meters]] That should not slow this down. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @iRevive you should just use a space there, not a |
||
*/ | ||
def meterProvider: MeterProvider[F] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
/* | ||
* Copyright 2022 Typelevel | ||
* | ||
* 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.typelevel.otel4s.meta | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the module name be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, is this file misplaced? Or maybe I'm very confused by the source reorganizaiton 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, sorry, ignore me 😂 |
||
|
||
import cats.Applicative | ||
|
||
trait InstrumentMeta[F[_]] { | ||
|
||
/** Indicates whether instrumentation is enabled or not. | ||
*/ | ||
def isEnabled: Boolean | ||
|
||
/** A no-op effect. | ||
*/ | ||
def unit: F[Unit] | ||
|
||
} | ||
|
||
object InstrumentMeta { | ||
|
||
def enabled[F[_]: Applicative]: InstrumentMeta[F] = | ||
make(enabled = true) | ||
|
||
def disabled[F[_]: Applicative]: InstrumentMeta[F] = | ||
make(enabled = false) | ||
|
||
private def make[F[_]: Applicative](enabled: Boolean): InstrumentMeta[F] = | ||
new InstrumentMeta[F] { | ||
val isEnabled: Boolean = enabled | ||
val unit: F[Unit] = Applicative[F].unit | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need full
IO
here, should it be ce-kernel or ce-std? Does it matter (it won't if we add fs2 deps). Do we care?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Actually,
cats-effect-kernel
is enough.I was thinking about
fs2
dependency intracing
module recently. Unless we want to introduce some deep tracing for stream scopes (I'm not even sure how plausible it is) thescodec
is more than enough.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I played with extending stream tracing to Natchez, and concluded that having a way to trace
Resource
was good enough for streams. All a stream dependency brings is a convenience method aroundStream.resource(whateverCreatesTheSpan)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, once a streaming scope is described as a
Resource
, the propagation works well with streams.otel4s/java/src/test/scala/org/typelevel/otel4s/java/trace/TracerSuite.scala
Lines 467 to 483 in d70a57d