From 2f024e55b3fd8c983e13219824dc596051dca655 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Mon, 6 May 2024 12:20:48 -0400 Subject: [PATCH 1/6] chore: bump version for next release (#888) --- VERSION | 2 +- axiom/nr_version.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/VERSION b/VERSION index ea4ccb283..b61c07ffd 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -10.20.0 +10.21.0 diff --git a/axiom/nr_version.c b/axiom/nr_version.c index 9cab7b786..18d948754 100644 --- a/axiom/nr_version.c +++ b/axiom/nr_version.c @@ -23,7 +23,6 @@ /* * Current version naming scheme is flowers * - * allium 14Mar2022 (9.20) * buttercup 26Apr2022 (9.21) * cosmos 29Jun2022 (10.0) * dahlia 19Sep2022 (10.1) @@ -45,8 +44,9 @@ * tulip 21Feb2024 (10.17) * ulmus 04Mar2024 (10.18) * viburnum 18Mar2024 (10.19) + * wallflower 06May2024 (10.20) */ -#define NR_CODENAME "wallflower" +#define NR_CODENAME "xerophyllum" const char* nr_version(void) { return NR_STR2(NR_VERSION); From 4d729291f7019c7248a26cc72535808fa3dc685d Mon Sep 17 00:00:00 2001 From: Hitesh Ahuja <108540135+hahuja2@users.noreply.github.com> Date: Wed, 8 May 2024 14:35:51 -0700 Subject: [PATCH 2/6] ci: update actions/upload-artifact and actions/download-artifact to v4 (#887) This PR updates `actions/upload-artifact` and `actions/download-artifact` to v4 which uses Node 20: actions/upload-artifact@v3 -> v4: https://github.com/actions/upload-artifact/releases/tag/v4.0.0 actions/download-artifact@v3 -> v4: https://github.com/actions/download-artifact/releases/tag/v4.0.0 --- .github/workflows/code-coverage-baseline.yml | 16 ++++++++-------- .github/workflows/make-agent.yml | 2 +- .github/workflows/make-for-platform-on-arch.yml | 2 +- .github/workflows/make-integration-tests.yml | 4 ++-- .github/workflows/release-build.yml | 8 ++++---- .github/workflows/test-agent.yml | 16 ++++++++-------- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/.github/workflows/code-coverage-baseline.yml b/.github/workflows/code-coverage-baseline.yml index fa4052e35..dcef64963 100644 --- a/.github/workflows/code-coverage-baseline.yml +++ b/.github/workflows/code-coverage-baseline.yml @@ -55,7 +55,7 @@ jobs: -v "${GITHUB_WORKSPACE}/php-agent":"/usr/local/src/newrelic-php-agent" $IMAGE_NAME:$IMAGE_TAG-${{ matrix.platform }}-$IMAGE_VERSION daemon_test - name: Save integration_runner for integration tests - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: integration_runner-${{matrix.platform}}-${{matrix.arch}} path: php-agent/bin/integration_runner @@ -124,19 +124,19 @@ jobs: -e ENABLE_COVERAGE=${{matrix.codecov}} $IMAGE_NAME:$IMAGE_TAG-${{matrix.php}}-${{matrix.platform}}-$IMAGE_VERSION make agent-check - name: Save newrelic.so for integration tests - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: newrelic.so-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/agent/modules/newrelic.so - name: Save axiom gcov data files (*.gcno, *.gcna) if: ${{ matrix.codecov == 1 }} - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: axiom.gcov-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/axiom/*.gc* - name: Save agent gcov data files (*.gcno, *.gcna) if: ${{ matrix.codecov == 1 }} - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: agent.gcov-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/agent/.libs/*.gc* @@ -161,24 +161,24 @@ jobs: repository: ${{ inputs.origin }}/newrelic-php-agent ref: ${{ inputs.ref }} - name: Get integration_runner - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: integration_runner-${{matrix.platform}}-${{matrix.arch}} path: php-agent/bin - name: Get newrelic.so - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: newrelic.so-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/agent/modules - name: Get axiom gcov data files if: ${{ matrix.codecov == 1 }} - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: axiom.gcov-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/axiom - name: Get agent gcov data files if: ${{ matrix.codecov == 1 }} - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: agent.gcov-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/agent/.libs diff --git a/.github/workflows/make-agent.yml b/.github/workflows/make-agent.yml index 87a473159..b29881676 100644 --- a/.github/workflows/make-agent.yml +++ b/.github/workflows/make-agent.yml @@ -62,7 +62,7 @@ jobs: -v "${GITHUB_WORKSPACE}/php-agent":"/usr/local/src/newrelic-php-agent" $IMAGE_NAME:$IMAGE_TAG-${{matrix.php}}-${{matrix.platform}}-$IMAGE_VERSION make agent - name: Save newrelic.so for integration tests - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: newrelic.so-${{matrix.platform}}-${{inputs.arch}}-${{matrix.php}} path: php-agent/agent/modules/newrelic.so diff --git a/.github/workflows/make-for-platform-on-arch.yml b/.github/workflows/make-for-platform-on-arch.yml index 053d833ff..17f1d544e 100644 --- a/.github/workflows/make-for-platform-on-arch.yml +++ b/.github/workflows/make-for-platform-on-arch.yml @@ -78,7 +78,7 @@ jobs: -e APP_supportability=${{secrets.APP_SUPPORTABILITY}} $IMAGE_NAME:$IMAGE_TAG-${{ matrix.platform }}-$IMAGE_VERSION ${{inputs.make-target}} - name: Save ${{inputs.make-target}} artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: ${{inputs.artifact-name}}-${{matrix.platform}}-${{inputs.arch}} path: php-agent/${{ inputs.artifact-pattern }} diff --git a/.github/workflows/make-integration-tests.yml b/.github/workflows/make-integration-tests.yml index 9e08e0b5e..800146f41 100644 --- a/.github/workflows/make-integration-tests.yml +++ b/.github/workflows/make-integration-tests.yml @@ -43,12 +43,12 @@ jobs: repository: ${{ inputs.origin }}/newrelic-php-agent ref: ${{ inputs.ref }} - name: Get integration_runner - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: integration_runner-${{matrix.platform}}-${{inputs.arch}} path: php-agent/bin - name: Get newrelic.so - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: newrelic.so-${{matrix.platform}}-${{inputs.arch}}-${{matrix.php}} path: php-agent/agent/modules diff --git a/.github/workflows/release-build.yml b/.github/workflows/release-build.yml index b90fc891a..84b74a107 100644 --- a/.github/workflows/release-build.yml +++ b/.github/workflows/release-build.yml @@ -52,10 +52,10 @@ jobs: -e BUILD_NUMBER=${{inputs.build-number}} $IMAGE_NAME:$IMAGE_TAG-${{ matrix.platform }}-$IMAGE_VERSION release-daemon - name: Save build artifacts - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: path: newrelic-php-agent/releases - name: release-from-gha + name: release-from-gha-${{ matrix.platform }}-${{ matrix.arch }} if-no-files-found: error agent_release: env: @@ -87,8 +87,8 @@ jobs: -e BUILD_NUMBER=${{inputs.build-number}} $IMAGE_NAME:agent-builder-php${{matrix.php_ver}}-${{matrix.platform}}-$IMAGE_VERSION release-${{matrix.php_ver}}-gha - name: Save build artifacts - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: path: newrelic-php-agent/releases - name: release-from-gha + name: release-from-gha-${{ matrix.platform }}-${{ matrix.arch }} if-no-files-found: error diff --git a/.github/workflows/test-agent.yml b/.github/workflows/test-agent.yml index 0365b4ddb..d40384e13 100644 --- a/.github/workflows/test-agent.yml +++ b/.github/workflows/test-agent.yml @@ -58,7 +58,7 @@ jobs: -v "${GITHUB_WORKSPACE}/php-agent":"/usr/local/src/newrelic-php-agent" $IMAGE_NAME:$IMAGE_TAG-${{ matrix.platform }}-$IMAGE_VERSION daemon_test - name: Save integration_runner for integration tests - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: integration_runner-${{matrix.platform}}-${{matrix.arch}} path: php-agent/bin/integration_runner @@ -159,19 +159,19 @@ jobs: -e ENABLE_COVERAGE=${{matrix.codecov}} $IMAGE_NAME:$IMAGE_TAG-${{matrix.php}}-${{matrix.platform}}-$IMAGE_VERSION make agent-${{ steps.get-check-variant.outputs.AGENT_CHECK_VARIANT }} - name: Save newrelic.so for integration tests - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: newrelic.so-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/agent/modules/newrelic.so - name: Save axiom gcov data files (*.gcno, *.gcna) if: ${{ matrix.codecov == 1 }} - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: axiom.gcov-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/axiom/*.gc* - name: Save agent gcov data files (*.gcno, *.gcna) if: ${{ matrix.codecov == 1 }} - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: agent.gcov-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/agent/.libs/*.gc* @@ -206,24 +206,24 @@ jobs: with: path: php-agent - name: Get integration_runner - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: integration_runner-${{matrix.platform}}-${{matrix.arch}} path: php-agent/bin - name: Get newrelic.so - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: newrelic.so-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/agent/modules - name: Get axiom gcov data files if: ${{ matrix.codecov == 1 }} - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: axiom.gcov-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/axiom - name: Get agent gcov data files if: ${{ matrix.codecov == 1 }} - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: agent.gcov-${{matrix.platform}}-${{matrix.arch}}-${{matrix.php}} path: php-agent/agent/.libs From d76a34bbc06c3de393614043904fc713a87f7d24 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Thu, 9 May 2024 12:21:26 -0400 Subject: [PATCH 3/6] fix(agent): end guzzle's external segment for rejected request (#883) Modify the onRejected handler to end guzzle's external segment even when there's no response. This allows to mark the span as an external span and capture the uri and method of the request. Additionally when sync requests is rejected, onRejected callback is not called via `PromiseInterface::then` and needs to be called manually. Fixes #320. --------- Co-authored-by: Amber Sistla --- agent/lib_guzzle6.c | 72 ++++---- daemon/cmd/integration_runner/main.go | 11 ++ daemon/internal/newrelic/integration/test.go | 33 ++-- .../newrelic/integration/test_test.go | 69 ++++++++ .../test_bad_response_exception_async.php | 92 ++++++++++ .../test_bad_response_exception_sync.php | 96 ++++++++++ .../external/guzzle6/test_timeout_async.php | 67 +++++++ .../external/guzzle6/test_timeout_sync.php | 67 +++++++ .../guzzle6/test_transfer_exception_async.php | 86 +++++++++ .../guzzle6/test_transfer_exception_sync.php | 90 ++++++++++ ...t_uncaught_bad_response_exception_sync.php | 165 ++++++++++++++++++ ...test_uncaught_transfer_exception_async.php | 161 +++++++++++++++++ .../test_bad_response_exception_async.php | 92 ++++++++++ .../test_bad_response_exception_sync.php | 96 ++++++++++ .../external/guzzle7/test_timeout_async.php | 67 +++++++ .../external/guzzle7/test_timeout_sync.php | 67 +++++++ .../guzzle7/test_transfer_exception_async.php | 86 +++++++++ .../guzzle7/test_transfer_exception_sync.php | 90 ++++++++++ ...t_uncaught_bad_response_exception_sync.php | 165 ++++++++++++++++++ ...test_uncaught_transfer_exception_async.php | 161 +++++++++++++++++ 20 files changed, 1787 insertions(+), 46 deletions(-) create mode 100644 tests/integration/external/guzzle6/test_bad_response_exception_async.php create mode 100644 tests/integration/external/guzzle6/test_bad_response_exception_sync.php create mode 100644 tests/integration/external/guzzle6/test_timeout_async.php create mode 100644 tests/integration/external/guzzle6/test_timeout_sync.php create mode 100644 tests/integration/external/guzzle6/test_transfer_exception_async.php create mode 100644 tests/integration/external/guzzle6/test_transfer_exception_sync.php create mode 100644 tests/integration/external/guzzle6/test_uncaught_bad_response_exception_sync.php create mode 100644 tests/integration/external/guzzle6/test_uncaught_transfer_exception_async.php create mode 100644 tests/integration/external/guzzle7/test_bad_response_exception_async.php create mode 100644 tests/integration/external/guzzle7/test_bad_response_exception_sync.php create mode 100644 tests/integration/external/guzzle7/test_timeout_async.php create mode 100644 tests/integration/external/guzzle7/test_timeout_sync.php create mode 100644 tests/integration/external/guzzle7/test_transfer_exception_async.php create mode 100644 tests/integration/external/guzzle7/test_transfer_exception_sync.php create mode 100644 tests/integration/external/guzzle7/test_uncaught_bad_response_exception_sync.php create mode 100644 tests/integration/external/guzzle7/test_uncaught_transfer_exception_async.php diff --git a/agent/lib_guzzle6.c b/agent/lib_guzzle6.c index 9038c2aaf..a106eac71 100644 --- a/agent/lib_guzzle6.c +++ b/agent/lib_guzzle6.c @@ -112,17 +112,13 @@ static void nr_guzzle6_requesthandler_handle_response(zval* handler, nr_segment_external_params_t external_params = {.library = "Guzzle 6"}; zval* request; zval* method; - zval* status; + zval* status = NULL; if (NR_FAILURE == nr_guzzle_obj_find_and_remove(handler, &segment TSRMLS_CC)) { return; } - if (!nr_php_psr7_is_response(response TSRMLS_CC)) { - return; - } - request = nr_guzzle6_requesthandler_get_request(handler TSRMLS_CC); if (NULL == request) { return; @@ -133,25 +129,6 @@ static void nr_guzzle6_requesthandler_handle_response(zval* handler, return; } - /* - * Get the X-NewRelic-App-Data response header. If there isn't one, NULL is - * returned, and everything still works just fine. - */ - external_params.encoded_response_header - = nr_php_psr7_message_get_header(response, X_NEWRELIC_APP_DATA TSRMLS_CC); - - if (NRPRG(txn) && NRTXN(special_flags.debug_cat)) { - nrl_verbosedebug( - NRL_CAT, "CAT: outbound response: transport='Guzzle 6' %s=" NRP_FMT, - X_NEWRELIC_APP_DATA, NRP_CAT(external_params.encoded_response_header)); - } - - status = nr_php_call(response, "getStatusCode"); - - if (nr_php_is_zval_valid_integer(status)) { - external_params.status = Z_LVAL_P(status); - } - method = nr_php_call(request, "getMethod"); if (nr_php_is_zval_valid_string(method)) { @@ -159,6 +136,27 @@ static void nr_guzzle6_requesthandler_handle_response(zval* handler, = nr_strndup(Z_STRVAL_P(method), Z_STRLEN_P(method)); } + if (NULL != response && nr_php_psr7_is_response(response TSRMLS_CC)) { + /* + * Get the X-NewRelic-App-Data response header. If there isn't one, NULL is + * returned, and everything still works just fine. + */ + external_params.encoded_response_header + = nr_php_psr7_message_get_header(response, X_NEWRELIC_APP_DATA TSRMLS_CC); + + if (NRPRG(txn) && NRTXN(special_flags.debug_cat)) { + nrl_verbosedebug( + NRL_CAT, "CAT: outbound response: transport='Guzzle 6' %s=" NRP_FMT, + X_NEWRELIC_APP_DATA, NRP_CAT(external_params.encoded_response_header)); + } + + status = nr_php_call(response, "getStatusCode"); + + if (nr_php_is_zval_valid_integer(status)) { + external_params.status = Z_LVAL_P(status); + } + } + nr_segment_external_end(&segment, &external_params); nr_free(external_params.encoded_response_header); @@ -291,6 +289,12 @@ static PHP_NAMED_FUNCTION(nr_guzzle6_requesthandler_onrejected) { return; } + this_obj = NR_PHP_USER_FN_THIS(); + if (NULL == this_obj) { + nrl_verbosedebug(NRL_FRAMEWORK, "%s: cannot obtain 'this'", __func__); + return; + } + /* * See if this is an exception that we can get a response from. We're going * to look for BadResponseException because, although it inherits from @@ -305,6 +309,7 @@ static PHP_NAMED_FUNCTION(nr_guzzle6_requesthandler_onrejected) { */ if (!nr_php_object_instanceof_class( exc, "GuzzleHttp\\Exception\\BadResponseException" TSRMLS_CC)) { + nr_guzzle6_requesthandler_handle_response(this_obj, NULL); return; } @@ -314,12 +319,6 @@ static PHP_NAMED_FUNCTION(nr_guzzle6_requesthandler_onrejected) { return; } - this_obj = NR_PHP_USER_FN_THIS(); - if (NULL == this_obj) { - nrl_verbosedebug(NRL_FRAMEWORK, "%s: cannot obtain 'this'", __func__); - return; - } - nr_guzzle6_requesthandler_handle_response(this_obj, response TSRMLS_CC); nr_php_zval_free(&response); @@ -448,6 +447,7 @@ void nr_guzzle6_enable(TSRMLS_D) { "namespace newrelic\\Guzzle6;" "use Psr\\Http\\Message\\RequestInterface;" + "use GuzzleHttp\\Promise\\PromiseInterface;" "if (!function_exists('newrelic\\Guzzle6\\middleware')) {" " function middleware(callable $handler) {" @@ -468,8 +468,16 @@ void nr_guzzle6_enable(TSRMLS_D) { */ " $rh = new RequestHandler($request);" " $promise = $handler($request, $options);" - " $promise->then([$rh, 'onFulfilled'], [$rh, 'onRejected']);" - + " if (PromiseInterface::REJECTED == $promise->getState()) {" + /* + Special case for sync request. When sync requests is rejected, + onRejected callback is not called via `PromiseInterface::then` + and needs to be called manually. + */ + " $rh->onRejected($promise);" + " } else {" + " $promise->then([$rh, 'onFulfilled'], [$rh, 'onRejected']);" + " }" " return $promise;" " };" " }" diff --git a/daemon/cmd/integration_runner/main.go b/daemon/cmd/integration_runner/main.go index f555ffab5..40f472af4 100644 --- a/daemon/cmd/integration_runner/main.go +++ b/daemon/cmd/integration_runner/main.go @@ -245,6 +245,16 @@ func catRequest(w http.ResponseWriter, r *http.Request) { w.Write(body) } +func delayRequest(w http.ResponseWriter, r *http.Request) { + duration := r.URL.Query().Get("duration") + io.WriteString(w, "waiting...") + d, err := time.ParseDuration(duration) + if nil != err { + d = 0 + } + time.Sleep(d) +} + func init() { //setup typed flags flag.Var(&flagLicense, "license", "use a license key other than the hard coded default. Supports @filename syntax for loading from files.") @@ -303,6 +313,7 @@ func main() { io.WriteString(w, "Hello world!") }) mux.HandleFunc("/cat", catRequest) + mux.HandleFunc("/delay", delayRequest) addr := "127.0.0.1:" + strconv.Itoa(*flagExternalPort) srv := &http.Server{Addr: addr, Handler: mux} ln, err := net.Listen("tcp", addr) diff --git a/daemon/internal/newrelic/integration/test.go b/daemon/internal/newrelic/integration/test.go index 37e55f09c..478c349fc 100644 --- a/daemon/internal/newrelic/integration/test.go +++ b/daemon/internal/newrelic/integration/test.go @@ -102,7 +102,7 @@ type Test struct { } func NewTest(name string) *Test { - test := &Test{Name: name} + test := &Test{Name: name, Env: make(map[string]string)} test.SetExpectHarvest(true) return test } @@ -200,14 +200,14 @@ func merge(a, b map[string]string) map[string]string { } func (t *Test) MakeRun(ctx *Context) (Tx, error) { - env := merge(ctx.Env, t.Env) + t.Env = merge(ctx.Env, t.Env) settings := merge(ctx.Settings, t.Settings) settings["newrelic.appname"] = t.Name headers := make(http.Header) for key, vals := range t.headers { for _, v := range vals { - expanded := SubEnvVars([]byte(v)) + expanded := t.subEnvVars([]byte(v)) headers.Set(key, string(expanded)) } } @@ -215,12 +215,12 @@ func (t *Test) MakeRun(ctx *Context) (Tx, error) { settings = merge(settings, t.PhpModules) if t.IsC() { - return CTx(ScriptFile(t.Path), env, settings, headers, ctx) + return CTx(ScriptFile(t.Path), t.Env, settings, headers, ctx) } if t.IsWeb() { - return CgiTx(ScriptFile(t.Path), env, settings, headers, ctx) + return CgiTx(ScriptFile(t.Path), t.Env, settings, headers, ctx) } - return PhpTx(ScriptFile(t.Path), env, settings, ctx) + return PhpTx(ScriptFile(t.Path), t.Env, settings, ctx) } func (t *Test) MakeSkipIf(ctx *Context) (Tx, error) { @@ -228,7 +228,7 @@ func (t *Test) MakeSkipIf(ctx *Context) (Tx, error) { return nil, nil } - env := merge(ctx.Env, t.Env) + t.Env = merge(ctx.Env, t.Env) settings := merge(ctx.Settings, t.Settings) settings["newrelic.appname"] = "skipif" @@ -239,7 +239,7 @@ func (t *Test) MakeSkipIf(ctx *Context) (Tx, error) { data: t.rawSkipIf, } - return PhpTx(src, env, settings, ctx) + return PhpTx(src, t.Env, settings, ctx) } type Scrub struct { @@ -285,10 +285,15 @@ func ScrubHost(in []byte) []byte { return re.ReplaceAll(in, []byte("__HOST__")) } -func SubEnvVars(in []byte) []byte { +func (t *Test) subEnvVars(in []byte) []byte { re := regexp.MustCompile("ENV\\[.*?\\]") return re.ReplaceAllFunc(in, func(match []byte) []byte { - return []byte(os.Getenv(string(match[4 : len(match)-1]))) + k := string(match[4 : len(match)-1]) + v, present := t.Env[k] + if !present { + v = os.Getenv(k) + } + return []byte(v) }) } @@ -345,7 +350,7 @@ func (t *Test) compareSpanEventsLike(harvest *newrelic.Harvest) { return } - if err := json.Unmarshal(t.spanEventsLike, &x2); nil != err { + if err := json.Unmarshal(t.subEnvVars(t.spanEventsLike), &x2); nil != err { t.Fatal(fmt.Errorf("unable to parse expected spans like json for fuzzy matching: %v", err)) return } @@ -497,7 +502,7 @@ func (t *Test) comparePayload(expected json.RawMessage, pc newrelic.PayloadCreat } } - expected = SubEnvVars(expected) + expected = t.subEnvVars(expected) err = IsFuzzyMatch(expected, actual) if nil != err { @@ -679,7 +684,7 @@ func (t *Test) Compare(harvest *newrelic.Harvest) { } if nil != t.metricsExist { - for _, name := range strings.Split(strings.TrimSpace(string(t.metricsExist)), "\n") { + for _, name := range strings.Split(strings.TrimSpace(string(t.subEnvVars(t.metricsExist))), "\n") { name = strings.TrimSpace(name) actual := strings.Replace(name, "__FILE__", t.Path, -1) if !harvest.Metrics.Has(actual) { @@ -689,7 +694,7 @@ func (t *Test) Compare(harvest *newrelic.Harvest) { } if nil != t.metricsDontExist { - for _, name := range strings.Split(strings.TrimSpace(string(t.metricsDontExist)), "\n") { + for _, name := range strings.Split(strings.TrimSpace(string(t.subEnvVars(t.metricsDontExist))), "\n") { name = strings.TrimSpace(name) actual := strings.Replace(name, "__FILE__", t.Path, -1) if harvest.Metrics.Has(actual) { diff --git a/daemon/internal/newrelic/integration/test_test.go b/daemon/internal/newrelic/integration/test_test.go index 582109def..891730bfa 100644 --- a/daemon/internal/newrelic/integration/test_test.go +++ b/daemon/internal/newrelic/integration/test_test.go @@ -7,6 +7,7 @@ package integration import ( "bytes" + "maps" "testing" "github.com/newrelic/newrelic-php-agent/daemon/internal/newrelic/sysinfo" @@ -62,3 +63,71 @@ func TestScrubHost(t *testing.T) { } } } + +func TestSubsEnvVar(t *testing.T) { + k := "TEST_KEY" + v := "test_value" + test := NewTest("SubsEnvVar") + test.Env[k] = v + + tests := []struct { + in []byte + want []byte + }{ + { + in: []byte("External/ENV[" + k + "]/all"), + want: []byte("External/" + v + "/all"), + }, + { + in: []byte("External/ENV[NON_EXISTING_KEY]/all"), + want: []byte("External//all"), + }, + } + + for i, tt := range tests { + if got := test.subEnvVars(tt.in); !bytes.Equal(got, tt.want) { + t.Errorf("%d. Test.subsEnvVar(%q) = %q; want %q", i, tt.in, got, tt.want) + } + } +} + +func makeTestWithEnv(name string, e map[string]string) *Test { + t := NewTest(name) + if nil != e { + maps.Copy(t.Env, e) + } + return t +} + +func TestMerge(t *testing.T) { + m1_k := "OS_KEY" + m1_v := "os_value" + m2_k := "TEST_KEY" + m2_v := "test_value" + m1 := map[string]string{ + m1_k: m1_v, + } + m2 := map[string]string{ + m2_k: m2_v, + } + same_key_diff_val := map[string]string{ + m1_k: m2_v, + } + tests := []struct { + name string + m map[string]string + k string + want string + }{ + {name: "MergeEmptyMap", m: nil, k: m1_k, want: m1_v}, + {name: "MergeDiffMaps", m: m2, k: m1_k, want: m1_v}, + {name: "MergeMapWithSameKeys", m: same_key_diff_val, k: m1_k, want: m2_v}, + } + for i, tt := range tests { + mm := merge(m1, tt.m) + got := mm[tt.k] + if got != tt.want { + t.Errorf("%d. %s - got: %s; want %s", i, tt.name, got, tt.want) + } + } +} diff --git a/tests/integration/external/guzzle6/test_bad_response_exception_async.php b/tests/integration/external/guzzle6/test_bad_response_exception_async.php new file mode 100644 index 000000000..528d81c75 --- /dev/null +++ b/tests/integration/external/guzzle6/test_bad_response_exception_async.php @@ -0,0 +1,92 @@ + $stack, + +]); + +$promise = $client->sendAsync($request); +GuzzleHttp\Promise\Utils::settle($promise)->wait(); + +echo "all's well that ends well" . PHP_EOL; diff --git a/tests/integration/external/guzzle6/test_bad_response_exception_sync.php b/tests/integration/external/guzzle6/test_bad_response_exception_sync.php new file mode 100644 index 000000000..bed8d8b7b --- /dev/null +++ b/tests/integration/external/guzzle6/test_bad_response_exception_sync.php @@ -0,0 +1,96 @@ + $stack, + +]); + +try { + $response = $client->send($request); +} catch (GuzzleHttp\Exception\BadResponseException $e) { + echo "gotcha!". PHP_EOL; +} + +echo "all's well that ends well" . PHP_EOL; diff --git a/tests/integration/external/guzzle6/test_timeout_async.php b/tests/integration/external/guzzle6/test_timeout_async.php new file mode 100644 index 000000000..caafd4d68 --- /dev/null +++ b/tests/integration/external/guzzle6/test_timeout_async.php @@ -0,0 +1,67 @@ + "$EXTERNAL_HOST", + // Set a short timeout, shorter than delay duration in the request + 'timeout' => 0.01, +]); + +$promise = $client->getAsync('/delay?duration=100ms'); +GuzzleHttp\Promise\Utils::settle($promise)->wait(); diff --git a/tests/integration/external/guzzle6/test_timeout_sync.php b/tests/integration/external/guzzle6/test_timeout_sync.php new file mode 100644 index 000000000..8ae5428f1 --- /dev/null +++ b/tests/integration/external/guzzle6/test_timeout_sync.php @@ -0,0 +1,67 @@ + "$EXTERNAL_HOST", + // Set a short timeout, shorter than delay duration in the request + 'timeout' => 0.01, +]); + +$response = $client->get('/delay?duration=100ms'); + diff --git a/tests/integration/external/guzzle6/test_transfer_exception_async.php b/tests/integration/external/guzzle6/test_transfer_exception_async.php new file mode 100644 index 000000000..924f466d0 --- /dev/null +++ b/tests/integration/external/guzzle6/test_transfer_exception_async.php @@ -0,0 +1,86 @@ + $stack, +]); + +$promise = $client->sendAsync($request); +GuzzleHttp\Promise\Utils::settle($promise)->wait(); + +echo "all's well that ends well" . PHP_EOL; diff --git a/tests/integration/external/guzzle6/test_transfer_exception_sync.php b/tests/integration/external/guzzle6/test_transfer_exception_sync.php new file mode 100644 index 000000000..42387ae13 --- /dev/null +++ b/tests/integration/external/guzzle6/test_transfer_exception_sync.php @@ -0,0 +1,90 @@ + $stack, +]); + +try { + $response = $client->send($request); +} catch (\GuzzleHttp\Exception\TransferException $e) { + echo "gotcha!". PHP_EOL; +} + +echo "all's well that ends well" . PHP_EOL; diff --git a/tests/integration/external/guzzle6/test_uncaught_bad_response_exception_sync.php b/tests/integration/external/guzzle6/test_uncaught_bad_response_exception_sync.php new file mode 100644 index 000000000..9f9a28af7 --- /dev/null +++ b/tests/integration/external/guzzle6/test_uncaught_bad_response_exception_sync.php @@ -0,0 +1,165 @@ + $stack, + +]); + +$response = $client->send($request); + +echo "you should not see this" . PHP_EOL; diff --git a/tests/integration/external/guzzle6/test_uncaught_transfer_exception_async.php b/tests/integration/external/guzzle6/test_uncaught_transfer_exception_async.php new file mode 100644 index 000000000..b6643cb4f --- /dev/null +++ b/tests/integration/external/guzzle6/test_uncaught_transfer_exception_async.php @@ -0,0 +1,161 @@ + $stack, +]); + +$promise = $client->sendAsync($request); +$promise->wait(); + +echo "you should not see this" . PHP_EOL; diff --git a/tests/integration/external/guzzle7/test_bad_response_exception_async.php b/tests/integration/external/guzzle7/test_bad_response_exception_async.php new file mode 100644 index 000000000..2047d4b09 --- /dev/null +++ b/tests/integration/external/guzzle7/test_bad_response_exception_async.php @@ -0,0 +1,92 @@ + $stack, + +]); + +$promise = $client->sendAsync($request); +GuzzleHttp\Promise\Utils::settle($promise)->wait(); + +echo "all's well that ends well" . PHP_EOL; diff --git a/tests/integration/external/guzzle7/test_bad_response_exception_sync.php b/tests/integration/external/guzzle7/test_bad_response_exception_sync.php new file mode 100644 index 000000000..95cba3426 --- /dev/null +++ b/tests/integration/external/guzzle7/test_bad_response_exception_sync.php @@ -0,0 +1,96 @@ + $stack, + +]); + +try { + $response = $client->send($request); +} catch (\GuzzleHttp\Exception\BadResponseException $e) { + echo "gotcha!". PHP_EOL; +} + +echo "all's well that ends well" . PHP_EOL; diff --git a/tests/integration/external/guzzle7/test_timeout_async.php b/tests/integration/external/guzzle7/test_timeout_async.php new file mode 100644 index 000000000..3f062c554 --- /dev/null +++ b/tests/integration/external/guzzle7/test_timeout_async.php @@ -0,0 +1,67 @@ + "$EXTERNAL_HOST", + // Set a short timeout, shorter than delay duration in the request + 'timeout' => 0.01, +]); + +$promise = $client->getAsync('/delay?duration=100ms'); +GuzzleHttp\Promise\Utils::settle($promise)->wait(); diff --git a/tests/integration/external/guzzle7/test_timeout_sync.php b/tests/integration/external/guzzle7/test_timeout_sync.php new file mode 100644 index 000000000..f01ae9047 --- /dev/null +++ b/tests/integration/external/guzzle7/test_timeout_sync.php @@ -0,0 +1,67 @@ + "$EXTERNAL_HOST", + // Set a short timeout, shorter than delay duration in the request + 'timeout' => 0.01, +]); + +$response = $client->get('/delay?duration=100ms'); + diff --git a/tests/integration/external/guzzle7/test_transfer_exception_async.php b/tests/integration/external/guzzle7/test_transfer_exception_async.php new file mode 100644 index 000000000..40e2e794f --- /dev/null +++ b/tests/integration/external/guzzle7/test_transfer_exception_async.php @@ -0,0 +1,86 @@ + $stack, +]); + +$promise = $client->sendAsync($request); +GuzzleHttp\Promise\Utils::settle($promise)->wait(); + +echo "all's well that ends well" . PHP_EOL; diff --git a/tests/integration/external/guzzle7/test_transfer_exception_sync.php b/tests/integration/external/guzzle7/test_transfer_exception_sync.php new file mode 100644 index 000000000..196ec3273 --- /dev/null +++ b/tests/integration/external/guzzle7/test_transfer_exception_sync.php @@ -0,0 +1,90 @@ + $stack, +]); + +try { + $response = $client->send($request); +} catch (\GuzzleHttp\Exception\TransferException $e) { + echo "gotcha!". PHP_EOL; +} + +echo "all's well that ends well" . PHP_EOL; diff --git a/tests/integration/external/guzzle7/test_uncaught_bad_response_exception_sync.php b/tests/integration/external/guzzle7/test_uncaught_bad_response_exception_sync.php new file mode 100644 index 000000000..88e2acdaf --- /dev/null +++ b/tests/integration/external/guzzle7/test_uncaught_bad_response_exception_sync.php @@ -0,0 +1,165 @@ + $stack, + +]); + +$response = $client->send($request); + +echo "you should not see this" . PHP_EOL; diff --git a/tests/integration/external/guzzle7/test_uncaught_transfer_exception_async.php b/tests/integration/external/guzzle7/test_uncaught_transfer_exception_async.php new file mode 100644 index 000000000..56641dacd --- /dev/null +++ b/tests/integration/external/guzzle7/test_uncaught_transfer_exception_async.php @@ -0,0 +1,161 @@ + $stack, +]); + +$promise = $client->sendAsync($request); +$promise->wait(); + +echo "you should not see this" . PHP_EOL; From 48bf341809778272828857a4b5c6334f87a0304c Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Tue, 14 May 2024 16:32:08 -0400 Subject: [PATCH 4/6] chore(tests): pretty print actual metrics (#897) Enhance EXPECT_METRICS_{DONT_}_EXIST by pretty printing actual metrics on test failure. --- daemon/internal/newrelic/integration/test.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/daemon/internal/newrelic/integration/test.go b/daemon/internal/newrelic/integration/test.go index 478c349fc..0f5a23aea 100644 --- a/daemon/internal/newrelic/integration/test.go +++ b/daemon/internal/newrelic/integration/test.go @@ -686,9 +686,11 @@ func (t *Test) Compare(harvest *newrelic.Harvest) { if nil != t.metricsExist { for _, name := range strings.Split(strings.TrimSpace(string(t.subEnvVars(t.metricsExist))), "\n") { name = strings.TrimSpace(name) - actual := strings.Replace(name, "__FILE__", t.Path, -1) - if !harvest.Metrics.Has(actual) { - t.Fail(fmt.Errorf("metric does not exist: %s\n\nactual metric table: %s", actual, harvest.Metrics.DebugJSON())) + expected := strings.Replace(name, "__FILE__", t.Path, -1) + if !harvest.Metrics.Has(expected) { + actualPretty := bytes.Buffer{} + json.Indent(&actualPretty, []byte(harvest.Metrics.DebugJSON()), "", " ") + t.Fail(fmt.Errorf("metric does not exist: %s\n\nactual metric table: %s", expected, actualPretty.String())) } } } @@ -696,9 +698,11 @@ func (t *Test) Compare(harvest *newrelic.Harvest) { if nil != t.metricsDontExist { for _, name := range strings.Split(strings.TrimSpace(string(t.subEnvVars(t.metricsDontExist))), "\n") { name = strings.TrimSpace(name) - actual := strings.Replace(name, "__FILE__", t.Path, -1) - if harvest.Metrics.Has(actual) { - t.Fail(fmt.Errorf("unexpected metric in harvest: %s\n\nactual metric table: %s", actual, harvest.Metrics.DebugJSON())) + expected := strings.Replace(name, "__FILE__", t.Path, -1) + if harvest.Metrics.Has(expected) { + actualPretty := bytes.Buffer{} + json.Indent(&actualPretty, []byte(harvest.Metrics.DebugJSON()), "", " ") + t.Fail(fmt.Errorf("unexpected metric in harvest: %s\n\nactual metric table: %s", expected, actualPretty.String())) } } } From 3d09829a5e914b453bca2eb7be3d270e3ebf49c4 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Wed, 15 May 2024 09:30:36 -0400 Subject: [PATCH 5/6] chore(agent): cleanup agent's verbosedebug log (#896) Don't print `Stack depth: ...` messages. Stack depth can be inferred from the level of indentation used in `nr_php_show_exec` and `nr_php_show_exec_return` - both use `nr_php_show_exec_indentation`, which uses `NRPRG(php_cur_stack_depth)` to calculate indentation level. --- agent/php_execute.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/agent/php_execute.c b/agent/php_execute.c index 341502f6c..e4300d902 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -2163,9 +2163,6 @@ void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { int show_executes = NR_PHP_PROCESS_GLOBALS(special_flags).show_executes; if (nrunlikely(show_executes)) { - nrl_verbosedebug(NRL_AGENT, - "Stack depth: %d after OAPI function beginning via %s", - NRPRG(php_cur_stack_depth), __func__); nr_php_show_exec(NR_EXECUTE_ORIG_ARGS); } nr_php_instrument_func_begin(NR_EXECUTE_ORIG_ARGS); @@ -2191,9 +2188,6 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, = NR_PHP_PROCESS_GLOBALS(special_flags).show_execute_returns; if (nrunlikely(show_executes_return)) { - nrl_verbosedebug(NRL_AGENT, - "Stack depth: %d before OAPI function exiting via %s", - NRPRG(php_cur_stack_depth), __func__); nr_php_show_exec_return(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); } From 42136bbf1853f7f86b527e12c5cca99e49565168 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Wed, 15 May 2024 15:06:01 -0400 Subject: [PATCH 6/6] security(daemon): upgrade golang to 1.22 (#890) 1. Upgrade minimum toolchain required to build daemon to go1.22.3. 2. Remove `go.work` and `go.work.sum`, which as of go1.22 are no longer compatible with newrelic-php-agent's use of `vendor`. 3. Cleanup daemon's Makefile 4. Simplify CodeQL workflow --- .github/workflows/codeql.yml | 16 ++++------------ daemon/Makefile | 16 ++++------------ daemon/go.mod | 3 ++- go.work | 3 --- go.work.sum | 1 - 5 files changed, 10 insertions(+), 29 deletions(-) delete mode 100644 go.work delete mode 100644 go.work.sum diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 3c2334186..c715fe73b 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -41,6 +41,7 @@ jobs: uses: github/codeql-action/init@v3 with: languages: ${{ matrix.language }} + build-mode: manual # If you wish to specify custom queries, you can do so here or in a config file. # By default, queries listed here will override any specified in a config file. # Prefix the list here with "+" to use these queries and those in the config file. @@ -48,21 +49,12 @@ jobs: # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs # queries: security-extended,security-and-quality - - # Autobuild attempts to build any compiled languages (C/C++, C#, Go, Java, or Swift). - # If this step fails, then you should remove it and run the build manually (see below) - - name: Autobuild - uses: github/codeql-action/autobuild@v3 - # ℹī¸ Command-line programs to run using the OS shell. # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun - # If the Autobuild fails above, remove it and uncomment the following three lines. - # modify them (or add more) to build your code if your project, please refer to the EXAMPLE below for guidance. - - # - run: | - # echo "Run, Build Application using script" - # ./location_of_script_within_repo/buildscript.sh + - name: Build + run: | + make ${{ matrix.language == 'go' && 'daemon' || 'agent' }} - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v3 diff --git a/daemon/Makefile b/daemon/Makefile index 145d51147..6d2aca5fa 100644 --- a/daemon/Makefile +++ b/daemon/Makefile @@ -46,21 +46,13 @@ clean: clean-bin: $(GO) clean $(GOFLAGS) -i ./... -# setup depenencies for daemon -# THIS IS TEMPORARY NEEDS TO BE REPLACED BY PINNED VENDOR DEPENDENCIES -# will download vendor dependencies -.PHONY: go-setup-dependencies -go-setup-dependencies: - $(GO) mod tidy - $(GO) mod vendor - # Build the binaries .PHONY: $(BINARIES) -$(BINARIES): clean-bin go-setup-dependencies +$(BINARIES): clean-bin $(GO) install $(GOFLAGS) $(GO_MODULE)/cmd/$@ # the -race flag enabled the integrated Go race detector. Output to stderr. -race: clean-bin go-setup-dependencies +race: clean-bin $(GO) install -race $(GOFLAGS) $(GO_MODULE)/$@ # Test targets @@ -76,10 +68,10 @@ cover: rm -f $(DAEMON_COV_FILE) .PHONY: integration -integration: go-setup-dependencies +integration: $(GO) test $(GOFLAGS) -tags integration ./... .PHONY: test -test: go-setup-dependencies +test: $(GO) test $(GOFLAGS) ./... diff --git a/daemon/go.mod b/daemon/go.mod index 06d57f2a1..41f8c79ed 100644 --- a/daemon/go.mod +++ b/daemon/go.mod @@ -1,6 +1,7 @@ module github.com/newrelic/newrelic-php-agent/daemon -go 1.21.1 +go 1.21 +toolchain go1.22.3 require ( github.com/golang/protobuf v1.5.3 diff --git a/go.work b/go.work deleted file mode 100644 index 05e497662..000000000 --- a/go.work +++ /dev/null @@ -1,3 +0,0 @@ -go 1.21.1 - -use ./daemon diff --git a/go.work.sum b/go.work.sum deleted file mode 100644 index 207fd9712..000000000 --- a/go.work.sum +++ /dev/null @@ -1 +0,0 @@ -google.golang.org/genproto v0.0.0-20231106174013-bbf56f31fb17 h1:wpZ8pe2x1Q3f2KyT5f8oP/fa9rHAKgFPr/HZdNuS+PQ=