Skip to content
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

Derive outcome from the HTTP status code when available #4165

Merged
merged 9 commits into from
Sep 8, 2020

Conversation

jalvz
Copy link
Contributor

@jalvz jalvz commented Sep 7, 2020

Motivation/summary

Avoid big empty charts in Kibana for error rates when user have ~old agents

Checklist

I have considered changes for:

How to test these changes

Related issues

Closes #4164

@jalvz
Copy link
Contributor Author

jalvz commented Sep 7, 2020

Missing spec update (coming soon)

Spec: elastic/apm#338

@apmmachine
Copy link
Contributor

apmmachine commented Sep 7, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #4165 updated]

  • Start Time: 2020-09-08T09:16:51.111+0000

  • Duration: 44 min 47 sec

Test stats 🧪

Test Results
Failed 0
Passed 3273
Skipped 145
Total 3418

Steps errors

Expand to view the steps failures

  • Name: Compress

    • Description: tar --exclude=coverage-files.tgz -czf coverage-files.tgz coverage

    • Duration: 0 min 0 sec

    • Start Time: 2020-09-08T09:32:57.735+0000

    • log

  • Name: Compress

    • Description: tar --exclude=system-tests-linux-files.tgz -czf system-tests-linux-files.tgz system-tests

    • Duration: 0 min 0 sec

    • Start Time: 2020-09-08T09:45:52.445+0000

    • log

  • Name: Test Sync

    • Description: ./script/jenkins/sync.sh

    • Duration: 3 min 50 sec

    • Start Time: 2020-09-08T09:26:59.513+0000

    • log

Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just a few minor things.

model/modeldecoder/transaction_test.go Show resolved Hide resolved
model/modeldecoder/span_test.go Outdated Show resolved Hide resolved
model/modeldecoder/span_test.go Outdated Show resolved Hide resolved
model/modeldecoder/span_test.go Outdated Show resolved Hide resolved
model/modeldecoder/span_test.go Outdated Show resolved Hide resolved
model/modeldecoder/span_test.go Outdated Show resolved Hide resolved
@axw
Copy link
Member

axw commented Oct 15, 2020

Tested BC1 using the Go agent v1.8.0, which does not send outcome.

package main

import (
        "net/http"

        "go.elastic.co/apm"
)

func main() {
        tracer := apm.DefaultTracer
        tx := tracer.StartTransaction("name", "type")
        span := tx.StartSpan("name", "type", nil)
        req, _ := http.NewRequest("GET", "http://testing.invalid", nil)
        span.Context.SetHTTPRequest(req)
        span.Context.SetHTTPStatusCode(200)
        span.End()
        tx.Context.SetHTTPStatusCode(400)
        tx.End()
        tracer.Flush(nil)
}

As expected:

  • With transaction's HTTP status code set to 400, event.outcome is "success"
  • With transaction's HTTP status code set to 500, event.outcome is "failure"
  • With span's HTTP status code set to 200, event.outcome is "success"
  • With span's HTTP status code set to 400, event.outcome is "failure"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement fallback logic for event outcome to support older agents
3 participants