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

OtelExporter does not export Span Marks #1047

Closed
vishramachandran opened this issue Jun 25, 2021 · 5 comments
Closed

OtelExporter does not export Span Marks #1047

vishramachandran opened this issue Jun 25, 2021 · 5 comments

Comments

@vishramachandran
Copy link

vishramachandran commented Jun 25, 2021

Thanks for adding the new OpenTelemetry exporter.
I was testing it for our use, and found that it does not propagate Kamon span's Mark events as OTel span Event objects.

This is an easy change. I would have sent a PR out, but I am restricted by my employer. Could this easy fix go in soon?
Backport into 2.1 release would be immensely useful. 🙏

cc @SimunKaracic @pnerg

@pnerg
Copy link
Contributor

pnerg commented Jun 28, 2021

Did a quick check of the OTEL data model with respect to events.
I'm unfamiliar with Kamons Mark so I'm a bit unsure how to translate it to an event.

The event data model states

  message Event {
    // time_unix_nano is the time the event occurred.
    fixed64 time_unix_nano = 1;

    // name of the event.
    // This field is semantically required to be set to non-empty string.
    string name = 2;

    // attributes is a collection of attribute key/value pairs on the event.
    repeated opentelemetry.proto.common.v1.KeyValue attributes = 3;

    // dropped_attributes_count is the number of dropped attributes. If the value is 0,
    // then no attributes were dropped.
    uint32 dropped_attributes_count = 4;
  }

I'm assuming something in the lines of this would be the correct translation.
The key from the mark would match the name of the event.

  private[otel] def toProtoEvent(mark:Span.Mark):ProtoSpan.Event = {
    ProtoSpan.Event.newBuilder()
      .setName(mark.key)
      .setTimeUnixNano(toEpocNano(mark.instant))
      .build()
  }

@pnerg
Copy link
Contributor

pnerg commented Jun 28, 2021

As for your company having a policy dictating you're not allowed to provide a PR I'd say it's a really crappy policy.
If a company allows itself to use open-source but cannot consider providing anything back then I personally find that leaching.

I'm not affiliated with Kamon in any way more than having written a few tickets and provided a few PR's.
Using my private github account when doing work on github, though the companies I've worked with over the years have been fine with that and why wouldn't they?

@pnerg
Copy link
Contributor

pnerg commented Jun 28, 2021

I've added PR, @vishramachandran have a check and see if it's correctly mapped from Mark to Event

@vishramachandran
Copy link
Author

@pnerg Looks great. Thanks much for taking the time.

The restrictions in my company are legal barriers to ensure that copyright and trade secrets are not leaked. There is an approval process I need to go through for each repo, and it takes a while. As much as I would like to voice my opinion and discuss whether this is good or not, this is not the forum to do that, and I would like to respectfully bow out of that discussion.

SimunKaracic added a commit that referenced this issue Jul 5, 2021
added missing span marks to otel exporter #1047
@SimunKaracic
Copy link
Contributor

Fixed with v2.2.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants