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

Implement tracing API #37

Merged
merged 52 commits into from
Jan 23, 2023
Merged

Implement tracing API #37

merged 52 commits into from
Jan 23, 2023

Conversation

iRevive
Copy link
Contributor

@iRevive iRevive commented Jul 30, 2022

The implementation of the tracing API using the OpenTelemetry java library.

Notable changes:

  • The methods from the Span trait moved to the Scala-specific SpanMacro
  • Preserved laziness via macro
  • The tracing scope is stored in the IOLocal

Currently, the scope is stored in IOLocal. This approach has several drawbacks:

@iRevive iRevive changed the title Implement tracing API Draft: Implement tracing API Jul 30, 2022
@iRevive iRevive mentioned this pull request Jul 30, 2022
7 tasks
# Conflicts:
#	.github/workflows/ci.yml
#	build.sbt
#	core/tracing/src/main/scala/org/typelevel/otel4s/trace/SamplingDecision.scala
#	core/tracing/src/main/scala/org/typelevel/otel4s/trace/Span.scala
#	core/tracing/src/main/scala/org/typelevel/otel4s/trace/SpanBuilder.scala
#	core/tracing/src/main/scala/org/typelevel/otel4s/trace/SpanContext.scala
#	core/tracing/src/main/scala/org/typelevel/otel4s/trace/SpanFinalizer.scala
#	core/tracing/src/main/scala/org/typelevel/otel4s/trace/Tracer.scala
@iRevive iRevive marked this pull request as ready for review October 17, 2022 16:07
@iRevive iRevive changed the title Draft: Implement tracing API Implement tracing API Oct 17, 2022
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

It seems to me that most of the implementation could be private[java], which would give us a lot more flexibility to refactor once bincompat constraints become a thing.

): SpanBuilder[F] =
copy(finalizationStrategy = strategy)

def createManual: F[Span[F]] = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this naming make sense since we do not have Span.Auto and Span.Manual anymore?

Since it's a builder, the create part somewhat makes sense. On the other hand, the side effect of a starting span is happening under the hood.

From my point of view, we should advertise the fully managed lifecycle of a span Resource[F, Span[F]] as a preferred option.

Some options:

  • start: Resource[F, Span[F]] and startUnmanaged: F[Span[F]]
  • create: Resource[F, Span[F]] and createUnmanaged: F[Span[F]]

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 I like start and startUnmanaged, for the verbal symmetry with end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I renamed the methods.

# Conflicts:
#	.github/workflows/ci.yml
#	build.sbt
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

This is wonderful!

I'm making Otel a top priority at $WORK. I would love to do a preview release this week so more people can start instrumenting and see what feels good and what still needs work.

@rossabaker
Copy link
Member

The module name is tracing and the package is trace. These should probably be the same?

@iRevive
Copy link
Contributor Author

iRevive commented Jan 23, 2023

The module name is tracing and the package is trace. These should probably be the same?

Good catch. I renamed sbt modules.

@iRevive iRevive merged commit c1e9cfd into typelevel:main Jan 23, 2023
@iRevive iRevive deleted the tracing-api-impl branch January 30, 2023 14:30
chr12c pushed a commit to chr12c/otel4s that referenced this pull request Jun 26, 2023
* Define tracing API

* Add `spanBuilder` to `Tracer`. Better naming of `SpanContext` members

* Rename `SamplingStrategy` -> `SamplingDecision`

* Update scaladoc. Add `SpanFinalizer`

* Fix scaladoc

* Implement tracing API

* Improve Scaladoc

* Improve Scaladoc

* Better phrasing

* Docs cleanup

* Properly handle 'noop' scope

* Hide implementation of `SpanFinalizer`

* Docs update

* Replace `asChildOf` with `childScope` and `withParent`

* Add `withParent` to `SpanBuilder`

* Replace `childOf` with `childScope` in `Tracer`

* Make `Span` always return `SpanContext`

* Test propagation of trace info over stream scopes

* Make `Span` always have a `SpanContext`

* Add `setAttribute` method to `Span`

* Use `IO.to` syntax

* Update layout

* Run `sbt githubWorkflowGenerate`

* Update layout

* Revert redundant changes

* Cleanup

* Run `sbt githubWorkflowGenerate`

* Update layout

* run `scalafixAll`

* Implement `Span#setAttribute` via macro

* Fix method name

* Update implementation

* Minor cleanup of `build.sbt`

* Remove duplicated `munit` dependency

* Discard statement instead of calling `void`

* Hide implementation details behind `package[java]`

* Rename `withAttribute` -> `addAttribute`

* Merge upstream

* Rename `SpanBuilder.create` -> `SpanBuilder.start`, `SpanBuilder.createManual` -> `SpanBuilder.startUnmanaged`

* Rename sbt modules `tracing` -> `trace`

Co-authored-by: Ross A. Baker <ross@rossabaker.com>
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.

3 participants