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

Minor documentation typos #9

Merged
merged 1 commit into from
Dec 3, 2023
Merged

Minor documentation typos #9

merged 1 commit into from
Dec 3, 2023

Conversation

abh
Copy link
Contributor

@abh abh commented Dec 2, 2023

No description provided.

@abh
Copy link
Contributor Author

abh commented Dec 2, 2023

I was getting Plack::Middleware::OpenTelemetry ready for a release and was trying to add some options for it, so really what I wanted to figure out was how to add attributes to the resource. :-)

I couldn't figure out how to do it in code without setting the ENV{}.

Following the documentation I adjusted, for example

my $resource = OpenTelemetry::SDK::Resource->new(attributes => {foo => 12311});

OpenTelemetry->tracer_provider =
  OpenTelemetry::SDK::Trace::TracerProvider->new(resource => $resource,);

then also requires me to copy / redo the work done in OpenTelemetry::SDK to setup the defaults.

Using $ENV{OTEL_RESOURCE_ATTRIBUTES} works in my example program; but in "real code" some of the attributes are difficult to get to in a BEGIN{} block, so another practical API would be nice.

(Or an example of how it already works if I'm just missing how to do it!)

@abh
Copy link
Contributor Author

abh commented Dec 2, 2023

I tried making the resource thing work by providing the option when creating the span, but best I can tell it gets lost somewhere (and then I ran out of time for today):

https://github.com/abh/Plack-Middleware-OpenTelemetry/blob/89e63a65187e6d80fcf9552c6958f5126a3c25c6/lib/Plack/Middleware/OpenTelemetry.pm#L40

@jjatria jjatria merged commit f14f910 into jjatria:main Dec 3, 2023
6 checks passed
@jjatria
Copy link
Owner

jjatria commented Dec 3, 2023

Thanks! I'll have to think a little about the resources, but the typos are well spotted 👍

@jjatria
Copy link
Owner

jjatria commented Dec 3, 2023

my $resource;
if (my $a = $self->resource_attributes) {
    my $resource = OpenTelemetry::SDK::Resource->new()
      ->merge(OpenTelemetry::SDK::Resource->new(empty => 1, attributes => $a));
}

In this example, the problem is that the new resource you create gets assigned to a new $resource that you declare within the scope of the if block. As soon as this block finishes, you are back to the $resource variable from the outer scope, which you never assigned to. Try removing the my inside the block.

Also: the empty parameter is undocumented because I'm not sure it's going to stay (that was me working around some Object::Pad limitations). What will stay is the empty constructor, so you should probably do

-OpenTelemetry::SDK::Resource->new(empty => 1, attributes => $a)
+OpenTelemetry::SDK::Resource->empty(attributes => $a)

Incidentally, your code will create a new resource every time the middleware is called, even though the resource attributes (I imagine) won't change.

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.

2 participants