Conversation
14150f9 to
f265ad5
Compare
f265ad5 to
3789cda
Compare
ChezCrawford
left a comment
There was a problem hiding this comment.
This is looking better! Left a few comments.
| } | ||
|
|
||
| func (e EventV2) GetRoutingKey() string { | ||
| func (e *EventV2) GetRoutingKey() string { |
There was a problem hiding this comment.
Any particular reason we made these pointer receivers?
There was a problem hiding this comment.
My thought process was this: AddCustomDetail needs to be passed a pointer reference because it's mutating one of the fields in the underlying struct (which I think is correct). My two options were to either make AddCustomDetail return a new event and keep the data immutable, which I didn't like as much as passing the pointer reference, and then from there I changed the rest of of the methods to take a pointer reference for consistency.
| EventVersion: eventsapi.EventVersion2, | ||
| } | ||
|
|
||
| eventV2 := eventsapi.EventV2{ |
There was a problem hiding this comment.
I'm not sure how much this is used by we might consider making a constructor inside eventsapi like so:
func NewEventContainer(event Event) *EventContainer {
jsonEvent, _ := json.Marshal(event)
return &EventContainer{
EventVersion: event.Version(),
EventData: jsonEvent,
}
}if we're ever actually doing this in production code.
There was a problem hiding this comment.
As of right now this is only done here in the test helper, should I write the constructor anyways or leave as is?
| cmd.Flags().StringVarP(&sendEvent.ClientURL, "client-url", "u", "", "Client URL") | ||
| cmd.Flags().StringToStringVarP(&customDetails, "field", "f", map[string]string{}, "Add given KEY=VALUE pair to the event details") | ||
|
|
||
| cmd.MarkFlagRequired("routing-key") |
There was a problem hiding this comment.
This should probably be service-key right?
| Long: `Queue up a trigger, acknowledge, or resolve V1 event to PagerDuty | ||
| using a backwards-compatible set of flags. | ||
|
|
||
| Required flags: "routing-key", "event-type"`, |
ChezCrawford
left a comment
There was a problem hiding this comment.
Good work @rafusel . Take a look at the final comments in cmd/send.go but this should be good to go after addressing those.
Description
This PR adds V1 events API support to the agent using
pdagent send, which previously sent a V2 event using backwards compatible flags.Changes
Eventinterface,Version()andAddCustomDetail(k string, v interface{})AddCustomDetailallowscmdutil.RunSendCommandto insert custom details without knowing the specific fields custom details resides in V1 and V2 eventsEventContainerthat implements anUnmarshalEventmethod which returns anEvent, the event must be stored inEventContainerso that the persistent queue can store both V1 and V2 eventsEventVersiontypeEventContainerhas fieldsEventVersion(which is the event version as theEventVersiontype) andEventData(which holds the event's data in ajson.RawMessage)Versionmethod in theEventinterfacesendandenqueueconstruct version specific events, and when they get sent to the server they are stored in theEventVersiontypeEventContainerthroughout, until a request must be made to PagerDuty, which then unmarshals the Event and gets the underlyingEventV1orEventV2type