Add alertmanager exporter implementation #28906
Conversation
mcube8
changed the title
|
|
||
| func (s *alertmanagerExporter) convertEventsToAlertPayload(events []*alertmanagerEvent) []model.Alert { | ||
|
|
||
| var payload []model.Alert |
There was a problem hiding this comment.
you can preassign the length of the payload based on the size of the events slice.
There was a problem hiding this comment.
Thank you for your review, I have removed chore from the pull request title and added a commit addressing your suggestions.
|
This should not be marked as a chore, and a changelog should be present. |
mcube8
changed the title
|
|
||
| msg, err := json.Marshal(payload) | ||
| if err != nil { | ||
| s.settings.Logger.Debug("error marshaling alert to JSON", zap.Error(err)) |
There was a problem hiding this comment.
When you are returning an error, avoid logging it. Otherwise, users will end up seeing the same message twice: here, and from the caller.
If you are NOT returning an error (ie, when the error isn't decisive to stop the processing), it's fine to log it.
exporter/alertmanagerexporter/go.mod
Outdated
| go.opentelemetry.io/collector/exporter v0.88.1-0.20231112151805-b570812b1e06 | ||
| go.opentelemetry.io/collector/pdata v1.0.0-rcv0017.0.20231112151805-b570812b1e06 | ||
| go.opentelemetry.io/collector/semconv v0.88.1-0.20231112151805-b570812b1e06 | ||
| go.opentelemetry.io/collector/component v0.89.0 |
There was a problem hiding this comment.
Sorry about this, but we have a new release happening right now: 0.90.0/1.0.0. I tried to update your PR to use that, but ran into some dependency issues. Once this PR is rebased and the CI is green, I'll merge this one.
There was a problem hiding this comment.
Thank you! I have updated the exporter to use the new release. One of the checks is failing due to a post error, will retry and update here.
There was a problem hiding this comment.
@jpkrohling All checks have passed on retry, thank you.
jpkrohling
force-pushed
the
am_exporter_impl
branch
from
8cb7120
to
ae67065
Compare
|
@mcube8, this is still missing a changelog entry (see the failing CI check). |
mcube8
force-pushed
the
am_exporter_impl
branch
from
938cd00
to
718a092
Compare
I have added it back, please check my new commit. Thanks! |
**Description:** Export Implementation of Alertmanager Exporter Second PR - exporter implementation **Link to tracking Issue:** [open-telemetry#23659](open-telemetry#23569) **Testing:** Unit tests for exporter implementation **Documentation:** Readme and Sample Configs to use Alertmanager exporter
Description: Export Implementation of Alertmanager Exporter
Second PR - exporter implementation
Link to tracking Issue: #23659
Testing: Unit tests for exporter implementation
Documentation: Readme and Sample Configs to use Alertmanager exporter