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

Add Last9 as destination #1828

Merged
merged 11 commits into from
Dec 2, 2024
Merged

Add Last9 as destination #1828

merged 11 commits into from
Dec 2, 2024

Conversation

prathamesh-sonpatki
Copy link
Contributor

@prathamesh-sonpatki prathamesh-sonpatki commented Nov 24, 2024

Add Last9 as a destination.

Last9 supports all telemetry signals Logs, Metrics and Traces

Copy link
Collaborator

@blumamir blumamir left a comment

Choose a reason for hiding this comment

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

Thank you @prathamesh-sonpatki

This looks great. added few small nits.

I tried to open an account in last9 to test the integration, but seems the OpenTelemetry receiver should be enabled on your side:

The OpenTelemetry endpoint is not enabled. Please request to proceed.
Request to enable OpenTelemetry endpoint ↗

docs/backends/last9.mdx Show resolved Hide resolved
common/config/last9.go Outdated Show resolved Hide resolved
destinations/logos/last9.svg Outdated Show resolved Hide resolved
@prathamesh-sonpatki
Copy link
Contributor Author

Thanks, @blumamir. I have taken the review comments. You will see the OpenTelemetry endpoint available in your Last9 account 👍🏽

Let me know if any other change is required.

Copy link
Collaborator

@blumamir blumamir left a comment

Choose a reason for hiding this comment

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

Thanks again @prathamesh-sonpatki

Tried the integration locally from your branch. needed to apply few fixes to make it work (added as PR comments).

After those fixes, I can see data in your system.

Question: the api endpoint: https://otlp.last9.io:443 - will it always be this constant value for all users? if so, perhaps we can remove it from the config or make it optional to reduce friction and required configuration?

2 issues I detected in Last9 while testing:

  • on boarding screen, if I click the copy button for Auth Header it copies the value of password which was a bit difficult to figure out.
  • when searching traces in grafana with the tempo data source, every trace I click results in this message: "failed to get trace with id: 2f103f9e25b3b8ab1c7d66bd2325bbc8 Status: 404 Not Found Body: {"error":"trace not found"}"

Thanks again for adding this integration

common/config/last9.go Outdated Show resolved Hide resolved
common/config/last9.go Outdated Show resolved Hide resolved
destinations/data/last9.yaml Outdated Show resolved Hide resolved
destinations/data/last9.yaml Show resolved Hide resolved
@prathamesh-sonpatki
Copy link
Contributor Author

@blumamir Thanks! I have taken care of the feedback :)

  1. Regarding the URL otlp.last9.io, it is not the same for everyone; it depends on the region selected when onboarding, so we should keep it.
  2. The 404 error - my bad, I had missed a config change during deployment; it should be fixed now!
  3. Thanks for feedback on onboarding, we will work on improving it

Thanks!

@blumamir
Copy link
Collaborator

blumamir commented Dec 2, 2024

@prathamesh-sonpatki looks great.

Few more issues I spotted in last 9 while testing:

  • When I search traces in grafana with tempo data source - I can see 4 services, but there suppose to be 5. the memebership service is missing. sent to to jaeger to verify and I can see this service in jaeger but not last9:
image image
  • When viewing traces in the "Traces" page, I can only see one service (frontent) which is the entry point, but all other services are missing:
image

@blumamir blumamir merged commit 36f258c into odigos-io:main Dec 2, 2024
28 checks passed
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