-
Notifications
You must be signed in to change notification settings - Fork 528
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
Set <event>.duration.us
field in ingest pipeline
#7261
Conversation
We'll set the <event>.duration.us fields in ingest.
Translate event.duration to <event>.duration.us for transaction and span events, and then drop event.duration.
This pull request does not have a backport label. Could you fix it @axw? 🙏
NOTE: |
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
|
The integration tests are failing, so we'll need to update the package before merging this. |
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.
Change and handling of breaking change LGTM
/test |
System test failure is unrelated. |
confirmed with 772e8c1
package main
import (
"context"
"fmt"
"os"
"time"
"go.elastic.co/apm"
)
func main() {
version := "undefined"
if len(os.Args) > 1 {
version = os.Args[1]
}
name := fmt.Sprintf("apm-server-%s", version)
tx := apm.DefaultTracer.StartTransaction(name, "type")
ctx := apm.ContextWithTransaction(context.Background(), tx)
span, ctx := apm.StartSpan(ctx, name, "type")
span.Duration = 0
span.End()
tx.Duration = 0
tx.End()
<-time.After(time.Second)
apm.DefaultTracer.Flush(nil)
fmt.Printf("%s: %+v\n", name, apm.DefaultTracer.Stats())
}
|
Motivation/summary
Update package
model
to no longer settransaction.duration.us
orspan.duration.us
, and instead setevent.duration
. Update our ingest pipelines to useevent.duration
to set the legacy fields, and then removeevent.duration
. This pipeline can be removed once when we are ready and able to update the APM UI to useevent.duration
directly.I have listed this as a breaking change in the top-level changelog, as it will have visible effects for anyone not using the Elasticsearch output or the packaged ingest pipeline. I have not done this in the integration package changelog, as I assume the ingest pipeline is always in use there.
Checklist
apmpackage
have been made)- [ ] Documentation has been updatedHow to test these changes
Send transactions and spans with zero and non-zero durations, and ensure they each have
transaction.duration.us
orspan.duration.us
set, and do not haveevent.duration
set.Related issues
Prerequisite of #5999
Part of #3565 and #4120