Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Trying to implement push OTLP exporter #91

Closed
egtwp opened this issue May 18, 2023 · 14 comments · Fixed by #93
Closed

Trying to implement push OTLP exporter #91

egtwp opened this issue May 18, 2023 · 14 comments · Fixed by #93

Comments

@egtwp
Copy link
Contributor

egtwp commented May 18, 2023

Hello team,

My colleagues and I are trying to export autometrics macro-generated metrics to an OTLP collector by implementing a push exporter based on the opentelemetry-rust SDK.

Unfortunately, our attempts were unsuccessful 😅. We hope that with your help, we can achieve our goal and perhaps contribute to the implementation of the idea discussed in Push Vs Pull #34.

From our understanding, when importing the autometrics crate with default features, the macro defaults to your OpenTelemetry tracker. According to the code and issue Make the metrics registry configurable #20, it calls opentelemetry_api::global::meter("") to declare and register every metric.
We then implemented a metric push controller from the opentelemetry-rust SDK, as shown in this example; The method set_meter_provider is called inside the build method of the opentelemetry_otlp::new_pipeline() constructor.

We were able to successfully push custom metrics to the OTLP collector, but we couldn't see the autometrics macro-generated ones!

We have published a basic example of our efforts, along with instructions on how to replicate our tests. We hope this will be useful for you or anyone else interested in implementing this feature to understand what we are trying to do.

Perhaps we are missing something from your or the OpenTelemetry SDK implementation...

Thank you for your time. If you decide to spend time on this, we are at your disposal for any clarification or further testing.

@egtwp egtwp changed the title Trying to implement push otlp exporter Trying to implement push OTLP exporter May 18, 2023
@IvanMerrill
Copy link

I'm not a rust person so can't help with the specifics regarding the SDK here. Two things I can say though:

  1. I opened a discussion on push Vs pull Here so if you have specific requirements for push please join the discussion!
  2. Pull via the OTEL collector using the Prometheus receiver works. I've tested it and will be writing something up soon. Obviously if push is required this doesn't help, but if you're trying to get it into OTLP format and pull is ok, you can use this.

Hope that helps!

@egtwp
Copy link
Contributor Author

egtwp commented May 19, 2023

Thank you for your comment, @IvanMerrill.
We're currently using OTEL collector to scrape /metrics endpoint and than push them elsewhere, but we have some serverless on-demand function for which, seeing the (early stage) release of the OTEL SDK, we are trying to get rid of this mechanism.
I’ve read your discussion on push vs pull and I’m also a fan of the push mechanism. I’ll try to give my 2 cents on that.
Thanks again for your help!

@emschwartz
Copy link
Contributor

Hi @egtwp I'm sorry this didn't work immediately as expected but thank you for the detailed write up and example code! I'm not sure what the issue is yet but I'm looking into it now.

@egtwp
Copy link
Contributor Author

egtwp commented May 19, 2023

Hello @emschwartz, many many thanks for your time!
Please don’t hesitate to ask for clarification or point out any errors you may find in my code.

@emschwartz
Copy link
Contributor

Ugh, that is an annoying issue. It turns out that the version of the opentelemetry_api crate used by autometrics must exactly match the one used by the code that calls set_meter_provider. If not, cargo will pull in both versions and the meter they are using will be different. If you downgrade your opentelemetry_api to 0.18, it'll work.

Because the versions are 0.x.x, cargo considers 0.18 and 0.19 fundamentally different.

I'm not sure what the best approach would be for this within the autometrics crate. We could try to make the dependencies more loose but if we specify it as opentelemetry_api = { version=">=0.18" }, there's a chance that a future version will break compatibility.

@emschwartz
Copy link
Contributor

Hmm. This also means that using a git dependency for opentelemetry_api won't work either, because cargo will still bring in two versions of the crate.

@egtwp
Copy link
Contributor Author

egtwp commented May 19, 2023

Thank you @emschwartz!

I created a new branch on our repo where I downgraded OTEL dependecies to 0.18 redo the tests. Metrics are now exported, but cargo doc fails because of an issue in opentelemetry-proto-0.1.0 crate fixed in newer release.

I think that major of this issue is due to the alpha stage of opentelemetry-rust crate (metric sdk in particular), 0.18 implementation is quite different from 0.19, that is again completely different from actual main branch of the repo, most of their "packages" are also in different locations.

For the same reason, I onestly can't see a simple way to implement a more loose dependencies in your crate.

Maybe implementing and re-exporting a minimum set of functionality could be a solution, at least for our case, but probably not the best to mantain for you.

Naturally, I’m still very interested in hearing any different solutions or ideas that you or the autometrics team may have. I’m also open to collaborating on any ideas to find a solution that works for everyone.

Thank you again for your help and support.

@emschwartz
Copy link
Contributor

Good points. I guess we just need to keep bumping the version to keep the latest autometrics up to date with the latest opentelemetry versions. #93

This unfortunately doesn't help with the git dependency and the fact that their main branch is different again from the published crates. Hopefully this will be a somewhat short-lived issue while they stabilize their APIs a bit more... 🤞

Also, I don't think that re-exporting the opentelemetry functionality is going to work very well in our case because the global statics its using are called by functions that they have scattered all throughout their crate APIs.

@emschwartz
Copy link
Contributor

Hi @egtwp, just wanted to follow up on this. We released autometrics v0.5, which includes the opentelemetry 0.19 dependencies. Could you try it again with this updated version and the otel 0.19 versions (not the git dependency)?

@egtwp
Copy link
Contributor Author

egtwp commented Jun 6, 2023

Hi @egtwp, just wanted to follow up on this. We released autometrics v0.5, which includes the opentelemetry 0.19 dependencies. Could you try it again with this updated version and the otel 0.19 versions (not the git dependency)?

Sure, working on it!

@egtwp
Copy link
Contributor Author

egtwp commented Jun 6, 2023

@emschwartz I updated our example repo with working example for both autometrics 0.5.0 and 0.4.1. Hope this help!

@emschwartz
Copy link
Contributor

That's great to hear! Maybe we can tweak it a bit and pull it into the examples folder here. Would you be interested in submitting a PR for that?

@egtwp
Copy link
Contributor Author

egtwp commented Jun 6, 2023

That's great to hear! Maybe we can tweak it a bit and pull it into the examples folder here. Would you be interested in submitting a PR for that?

I will try!
Please give any input/guide lines on how to proceed.

@emschwartz
Copy link
Contributor

Amazing!

I think it's pretty close already.

  • I'd remove the custom metrics so it's stripped down to the core
  • The readme should keep the instructions for running the collector
  • It would be good to have a bit of an explanation of what you should see at the end

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
3 participants