-
Notifications
You must be signed in to change notification settings - Fork 3
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 --opamp-api-url flag to unix install script #76
Conversation
1ef97b7
to
ea8312c
Compare
Blocked on #78 |
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.
Can you add some tests to demonstrate that we're at least writing the right urls to the config files?
@swiatekm-sumo I will endeavour to do so! |
ea8312c
to
972a890
Compare
Now, the darwin install script test has failed because:
I will try to re-run it to see if it is transient. |
The tests have now failed successfully. I will file an issue about the transient CI error. |
This commit adds a new flag to the install script, responsible for setting the OpAmp API URL. Previously, the OpAmp API URL was computed from the API URL. However, this did not work correctly in all scenarios. This change is related to a pending change in the agent and may need to be held back until the agent change is committed.
@swiatekm-sumo I modeled this after the other configuration overrides, they basically regex-replace a line of yaml with another line. I'm a little concerned about doing this by searching for the word "endpoint". But I'm not sure how to improve things without adding dependencies (yq, etc). Any thoughts on how to make this more robust? I'm not a very good bash programmer unfortunately. |
@swiatekm-sumo I believe this feature is finally working, please take another look. |
LGTM! |
This commit adds a new flag to the install script, responsible for setting the OpAmp API URL.
Previously, the OpAmp API URL was computed from the API URL. However, this did not work correctly in all scenarios.
This change is related to a pending change in the agent and may need to be held back until the agent change is committed.
See SumoLogic/sumologic-otel-collector#1519