Skip to content

Commit 5762120

Browse files
bduranleau-nrbduranleau-nr
committed
remove InstrumentHTTPHandleFuncLit
1 parent 4752c8b commit 5762120

3 files changed

Lines changed: 56 additions & 149 deletions

File tree

‎parser/manager.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func NewInstrumentationManager(pkgs []*decorator.Package, appName, agentVariable
8686

8787
// DetectDependencyIntegrations
8888
func (m *InstrumentationManager) DetectDependencyIntegrations() error {
89-
m.loadStatelessTracingFunctions(InstrumentMain, InstrumentHandleFunction, InstrumentHttpClient, CannotInstrumentHttpMethod, InstrumentGrpcDial, InstrumentGinFunction, InstrumentGrpcServerMethod, InstrumentHTTPHandleFuncLit, InstrumentRouteHandlerFuncLit)
89+
m.loadStatelessTracingFunctions(InstrumentMain, InstrumentHandleFunction, InstrumentHttpClient, CannotInstrumentHttpMethod, InstrumentGrpcDial, InstrumentGinFunction, InstrumentGrpcServerMethod, InstrumentRouteHandlerFuncLit)
9090
m.loadStatefulTracingFunctions(ExternalHttpCall, WrapNestedHandleFunction, InstrumentGrpcServer, InstrumentGinMiddleware)
9191
m.loadDependencyScans(FindGrpcServerObject)
9292
return nil

‎parser/netHTTP.go‎

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -355,36 +355,6 @@ func WrapNestedHandleFunction(manager *InstrumentationManager, stmt dst.Stmt, c
355355
return wasModified
356356
}
357357

358-
// verify the existence of the HandleFunc call
359-
// http.HandleFunc("/route", func(w, r){...})
360-
// _____^^^^^^^^^^
361-
func getHTTPHandleFunc(node dst.Node) *dst.CallExpr {
362-
switch v := node.(type) {
363-
case *dst.ExprStmt:
364-
call, ok := v.X.(*dst.CallExpr)
365-
if !ok {
366-
return nil
367-
}
368-
369-
ident, ok := call.Fun.(*dst.Ident)
370-
if !ok {
371-
return nil
372-
}
373-
374-
if ident.Path != codegen.HttpImportPath {
375-
return nil
376-
}
377-
378-
if ident.Name != "HandleFunc" {
379-
return nil
380-
}
381-
382-
return call
383-
}
384-
385-
return nil
386-
}
387-
388358
// extract the HTTP method type from the route handler declaration
389359
// r.Get("/routename", func(w,r){...})
390360
// __^^^
@@ -524,41 +494,3 @@ func InstrumentRouteHandlerFuncLit(manager *InstrumentationManager, c *dstutil.C
524494
codegen.PrependStatementToFunctionLit(fnLit, txn)
525495
manager.addImport(codegen.NewRelicAgentImportPath)
526496
}
527-
528-
// InstrumentHTTPHandleFuncLit adds instrumentation for http HandleFunc function literals
529-
// For an http Handler function literal to be considered, it must satisfy the following constraints:
530-
// 1. A call to HandleFunc
531-
// 2. A valid route name (/index, /route/literal, etc) defined as the first argument to the route method
532-
// 3. A function literal as the second argument to the route method
533-
// 4. An http.ResponseWriter and http.Request argument to the function literal
534-
//
535-
// If all constraints above are satisfied, an NR txn object is retreived via the request context, and
536-
// injected alongside a defer segment start with the segment name comprising of the HTTP method +":routename"
537-
func InstrumentHTTPHandleFuncLit(manager *InstrumentationManager, c *dstutil.Cursor) {
538-
callExpr := getHTTPHandleFunc(c.Node())
539-
540-
if callExpr == nil {
541-
return
542-
}
543-
544-
routeName, fnLit := getHTTPHandlerRouteName(callExpr)
545-
routeName, err := strconv.Unquote(routeName)
546-
if routeName == "" || fnLit == nil || err != nil {
547-
return
548-
}
549-
550-
reqArgName := getHTTPRequestArgNameFnLit(fnLit)
551-
if reqArgName == "" {
552-
return
553-
}
554-
555-
txn := codegen.TxnFromContext(codegen.DefaultTransactionVariable, codegen.HttpRequestContext(reqArgName))
556-
if txn == nil {
557-
return
558-
}
559-
560-
segmentName := "http.HandleFunc" + ":" + routeName
561-
codegen.PrependStatementToFunctionLit(fnLit, codegen.DeferSegment(segmentName, dst.NewIdent(codegen.DefaultTransactionVariable)))
562-
codegen.PrependStatementToFunctionLit(fnLit, txn)
563-
manager.addImport(codegen.NewRelicAgentImportPath)
564-
}

‎parser/netHTTP_test.go‎

Lines changed: 55 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,13 +1537,9 @@ import (
15371537
)
15381538
15391539
func main() {
1540-
http.HandleFunc("/literal", func(w http.ResponseWriter, r *http.Request) {
1541-
nrTxn := newrelic.FromContext(r.Context())
1542-
1543-
defer nrTxn.StartSegment("http.HandleFunc:/literal").End()
1544-
1540+
http.HandleFunc(newrelic.WrapHandleFunc(txn.Application(), "/literal", func(w http.ResponseWriter, r *http.Request) {
15451541
w.Write([]byte("Hello, literal"))
1546-
})
1542+
}))
15471543
http.ListenAndServe(":8080", nil)
15481544
}
15491545
`,
@@ -1576,13 +1572,9 @@ import (
15761572
)
15771573
15781574
func setup() {
1579-
http.HandleFunc("/setup", func(w http.ResponseWriter, r *http.Request) {
1580-
nrTxn := newrelic.FromContext(r.Context())
1581-
1582-
defer nrTxn.StartSegment("http.HandleFunc:/setup").End()
1583-
1575+
http.HandleFunc(newrelic.WrapHandleFunc(txn.Application(), "/setup", func(w http.ResponseWriter, r *http.Request) {
15841576
w.Write([]byte("Hello, setup literal"))
1585-
})
1577+
}))
15861578
}
15871579
15881580
func main() {
@@ -1592,29 +1584,17 @@ func main() {
15921584
`,
15931585
},
15941586
{
1595-
name: "HTTP function literal instrumentation mixed case",
1587+
name: "HTTP function literal instrumentation request object naming",
15961588
code: `package main
15971589
15981590
import (
15991591
"net/http"
16001592
)
16011593
1602-
func setup() {
1603-
http.HandleFunc("/setup", func(w http.ResponseWriter, r *http.Request) {
1604-
w.Write([]byte("Hello, setup literal"))
1605-
})
1606-
}
1607-
1608-
func declaredHandler(w http.ResponseWriter, r *http.Request) {
1609-
w.Write([]byte("Hello, declared"))
1610-
}
1611-
16121594
func main() {
1613-
http.HandleFunc("/index", func(w http.ResponseWriter, r *http.Request) {
1614-
w.Write([]byte("Hello, index"))
1595+
http.HandleFunc("/literal", func(w http.ResponseWriter, req *http.Request) {
1596+
w.Write([]byte("Hello, literal"))
16151597
})
1616-
setup()
1617-
http.HandleFunc("/declared", declaredHandler)
16181598
http.ListenAndServe(":8080", nil)
16191599
}
16201600
`,
@@ -1626,89 +1606,74 @@ import (
16261606
"github.com/newrelic/go-agent/v3/newrelic"
16271607
)
16281608
1629-
func setup() {
1630-
http.HandleFunc("/setup", func(w http.ResponseWriter, r *http.Request) {
1631-
nrTxn := newrelic.FromContext(r.Context())
1632-
1633-
defer nrTxn.StartSegment("http.HandleFunc:/setup").End()
1634-
1635-
w.Write([]byte("Hello, setup literal"))
1636-
})
1637-
}
1638-
1639-
func declaredHandler(w http.ResponseWriter, r *http.Request) {
1640-
w.Write([]byte("Hello, declared"))
1641-
}
1642-
16431609
func main() {
1644-
http.HandleFunc("/index", func(w http.ResponseWriter, r *http.Request) {
1645-
nrTxn := newrelic.FromContext(r.Context())
1646-
1647-
defer nrTxn.StartSegment("http.HandleFunc:/index").End()
1648-
1649-
w.Write([]byte("Hello, index"))
1650-
})
1651-
setup()
1652-
http.HandleFunc("/declared", declaredHandler)
1610+
http.HandleFunc(newrelic.WrapHandleFunc(txn.Application(), "/literal", func(w http.ResponseWriter, req *http.Request) {
1611+
w.Write([]byte("Hello, literal"))
1612+
}))
16531613
http.ListenAndServe(":8080", nil)
16541614
}
16551615
`,
16561616
},
1617+
}
1618+
1619+
for _, tt := range tests {
1620+
t.Run(tt.name, func(t *testing.T) {
1621+
defer panicRecovery(t)
1622+
got := testStatefulTracingFunction(t, tt.code, WrapNestedHandleFunction, true)
1623+
assert.Equal(t, tt.expect, got)
1624+
})
1625+
}
1626+
1627+
}
1628+
1629+
func TestInstrumentRouterFunctionLiteral(t *testing.T) {
1630+
tests := []struct {
1631+
name string
1632+
code string
1633+
expect string
1634+
}{
16571635
{
1658-
name: "HTTP function literal instrumentation request object naming",
1636+
name: "Router literal function instrumentation in main",
16591637
code: `package main
16601638
16611639
import (
16621640
"net/http"
1641+
1642+
chi "github.com/go-chi/chi/v5"
16631643
)
16641644
16651645
func main() {
1666-
http.HandleFunc("/literal", func(w http.ResponseWriter, req *http.Request) {
1667-
w.Write([]byte("Hello, literal"))
1646+
router := chi.NewRouter()
1647+
router.Get("/lit-main", func(w http.ResponseWriter, r *http.Request) {
1648+
w.Write([]byte("Hello, main"))
16681649
})
1669-
http.ListenAndServe(":8080", nil)
1650+
http.ListenAndServe(":8080", r)
16701651
}
16711652
`,
16721653
expect: `package main
16731654
16741655
import (
16751656
"net/http"
16761657
1658+
chi "github.com/go-chi/chi/v5"
16771659
"github.com/newrelic/go-agent/v3/newrelic"
16781660
)
16791661
16801662
func main() {
1681-
http.HandleFunc("/literal", func(w http.ResponseWriter, req *http.Request) {
1682-
nrTxn := newrelic.FromContext(req.Context())
1663+
router := chi.NewRouter()
1664+
router.Get("/lit-main", func(w http.ResponseWriter, r *http.Request) {
1665+
nrTxn := newrelic.FromContext(r.Context())
16831666
1684-
defer nrTxn.StartSegment("http.HandleFunc:/literal").End()
1667+
defer nrTxn.StartSegment("GET:/lit-main").End()
16851668
1686-
w.Write([]byte("Hello, literal"))
1669+
w.Write([]byte("Hello, main"))
16871670
})
1688-
http.ListenAndServe(":8080", nil)
1671+
http.ListenAndServe(":8080", r)
16891672
}
16901673
`,
16911674
},
1692-
}
1693-
1694-
for _, tt := range tests {
1695-
t.Run(tt.name, func(t *testing.T) {
1696-
defer panicRecovery(t)
1697-
got := testStatelessTracingFunction(t, tt.code, InstrumentHTTPHandleFuncLit)
1698-
assert.Equal(t, tt.expect, got)
1699-
})
1700-
}
1701-
1702-
}
1703-
1704-
func TestInstrumentRouterFunctionLiteral(t *testing.T) {
1705-
tests := []struct {
1706-
name string
1707-
code string
1708-
expect string
1709-
}{
17101675
{
1711-
name: "Router literal function instrumentation in main",
1676+
name: `Router function instrumentation mixed case`,
17121677
code: `package main
17131678
17141679
import (
@@ -1717,11 +1682,16 @@ import (
17171682
chi "github.com/go-chi/chi/v5"
17181683
)
17191684
1685+
func declared(w http.ResponseWriter, r *http.Request) {
1686+
w.Write([]byte("declared"))
1687+
}
1688+
17201689
func main() {
17211690
router := chi.NewRouter()
1722-
router.GET("/lit-main", func(w http.ResponseWriter, r *http.Request) {
1691+
router.Get("/lit-main", func(w http.ResponseWriter, r *http.Request) {
17231692
w.Write([]byte("Hello, main"))
17241693
})
1694+
router.Get("/declared", declared)
17251695
http.ListenAndServe(":8080", r)
17261696
}
17271697
`,
@@ -1734,15 +1704,20 @@ import (
17341704
"github.com/newrelic/go-agent/v3/newrelic"
17351705
)
17361706
1707+
func declared(w http.ResponseWriter, r *http.Request) {
1708+
w.Write([]byte("declared"))
1709+
}
1710+
17371711
func main() {
17381712
router := chi.NewRouter()
1739-
router.GET("/lit-main", func(w http.ResponseWriter, r *http.Request) {
1713+
router.Get("/lit-main", func(w http.ResponseWriter, r *http.Request) {
17401714
nrTxn := newrelic.FromContext(r.Context())
17411715
17421716
defer nrTxn.StartSegment("GET:/lit-main").End()
17431717
17441718
w.Write([]byte("Hello, main"))
17451719
})
1720+
router.Get("/declared", declared)
17461721
http.ListenAndServe(":8080", r)
17471722
}
17481723
`,

0 commit comments

Comments
 (0)