-
Notifications
You must be signed in to change notification settings - Fork 41
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
Use sumologic extension base URL to determine the appropriate OpAMP endpoint #1399
Conversation
I feel this approach is a bit hacky and could fail if the URL structure of the base URL changes. However, given the transitional nature of the landscape here, perhaps it's fine. What do reviewers think? |
This is very hacky and I wouldn't say reliable as long-term solution. Can we ensure that this will be done properly in the future? The address should be returned by the API backend |
{ | ||
name: "empty url defaults to config endpoint", | ||
wantEndpoint: "https://example.com", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure that this test case indicates valid behaviour. It's a reproduction of the business logic that was encoded in the draft PR, and may need to be changed.
@swiatekm-sumo if you could take a look, I would appreciate it, thank you! 🙏 |
u.Host = strings.Replace(u.Host, "open-events", "opamp-events", 1) | ||
u.Host = strings.Replace(u.Host, "open-collectors", "opamp-collectors", 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we'd agreed to parse the Sumo deployment name out of the domain and explicitly construct a new one. Any reason to do a string replace here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's my naivete regarding Sumo deployments, but it seems even more error prone to me. How do I know which deployments correspond to open-events, and which correspond to open-collectors (and correspondingly, opamp-events and opamp-collectors). This knowledge is not encoded anywhere in the codebase, unless I'm mistaken, and I don't know how it would be determined either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping to do https://long-open-events.sumologic.net
-> deployment = long
-> construct wss://{{ deployment }}-opamp-events.sumologic.net/v1/opamp
. But looking at it now, these domains are actually different in production environments, so this might be even more complicated.
I'm fine with it if we add test cases for all the possible forms of this url. For example, on prod it's just open-collectors.sumologic.com
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wss://stag-opamp-events.sumologic.net/v1/opamp
wss://long-opamp-events.sumologic.net/v1/opamp
wss://opamp-collectors.au.sumologic.comt/v1/opamp
wss://opamp-collectors.ca.sumologic.comt/v1/opamp
wss://opamp-collectors.de.sumologic.comt/v1/opamp
wss://opamp-collectors.eu.sumologic.comt/v1/opamp
wss://opamp-collectors.fed.sumologic.comt/v1/opamp
wss://opamp-collectors.in.sumologic.comt/v1/opamp
wss://opamp-collectors.jp.sumologic.comt/v1/opamp
wss://opamp-collectors.sumologic.comt/v1/opamp
wss://opamp-collectors.us2.sumologic.comt/v1/opamp
Open receiver can be found at
https://help.sumologic.com/docs/api/getting-started/#sumo-logic-endpoints-by-deployment-and-firewall-security
3465f7f
to
e52be4c
Compare
Signed-off-by: Sean Porter <[email protected]>
Signed-off-by: Sean Porter <[email protected]>
Signed-off-by: Eric Chlebek <[email protected]>
Also upgrade sumologicextension module Signed-off-by: Eric Chlebek <[email protected]>
Signed-off-by: Eric Chlebek <[email protected]>
Signed-off-by: Eric Chlebek <[email protected]>
Signed-off-by: Eric Chlebek <[email protected]>
e52be4c
to
18555a9
Compare
9b5af12
to
e632091
Compare
The sumologic extension may update/alter the API base URL used, following the collector registration process. This base URL redirection is tied to the installation token which the OpAMP extension isn't aware of. Update the OpAMP extension to leverage the sumologic extension's base URL to determine the appropriate OpAMP endpoint (i.e. region).