feat(agent): Memcached instance metrics with host name

agent/Makefile.frag

@@ -93,6 +93,7 @@ TEST_BINARIES = \
 	tests/test_internal_instrument \
 	tests/test_hash \
 	tests/test_lib_aws_sdk_php \
+	tests/test_memcached \
 	tests/test_mongodb \
 	tests/test_monolog \
 	tests/test_mysql \

agent/config.m4

@@ -215,7 +215,7 @@ if test "$PHP_NEWRELIC" = "yes"; then
   php_error.c php_execute.c php_explain.c php_explain_mysqli.c \
   php_explain_pdo_mysql.c php_extension.c php_file_get_contents.c \
   php_globals.c php_hash.c php_header.c php_httprequest_send.c \
-  php_internal_instrument.c php_minit.c php_mshutdown.c php_mysql.c \
+  php_internal_instrument.c php_memcached.c php_minit.c php_mshutdown.c php_mysql.c \
   php_mysqli.c php_newrelic.c php_nrini.c php_observer.c php_output.c php_pdo.c \
   php_pdo_mysql.c php_pdo_pgsql.c php_pgsql.c php_psr7.c php_redis.c \
   php_rinit.c php_rshutdown.c php_samplers.c php_stack.c \

agent/php_internal_instrument.c

@@ -13,8 +13,10 @@
 #include "php_explain_mysqli.h"
 #include "php_file_get_contents.h"
 #include "php_globals.h"
+#include "php_hash.h"
 #include "php_httprequest_send.h"
 #include "php_internal_instrument.h"
+#include "php_memcached.h"
 #include "php_mysql.h"
 #include "php_mysqli.h"
 #include "php_pdo.h"
@@ -1531,6 +1533,57 @@ NR_INNER_WRAPPER(memcache_function) {
                                              INTERNAL_FUNCTION_PARAM_PASSTHRU);
 }
 
+NR_INNER_WRAPPER(memcached_add_server) {
+  char* host = NULL;
+  nr_string_len_t host_len = 0;
+  zend_long port = 0;
+  zend_long weight = 0;
+  int zcaught = 0;
+
+  if (SUCCESS
+      == zend_parse_parameters_ex(
+          ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "s|ll", &host,
+          &host_len, &port, &weight) &&
+      NULL != host) {
+    nr_php_memcached_create_instance_metric(host, port);
+  }
+  zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler,
+                                     INTERNAL_FUNCTION_PARAM_PASSTHRU);
+  if (zcaught) {
+    zend_bailout();
+    /* NOTREACHED */
+  }
+}
+
+NR_INNER_WRAPPER(memcached_add_servers) {
+  zval* servers = NULL;
+  zval* server = NULL;
+  int zcaught = 0;
+
+  if (SUCCESS
+      == zend_parse_parameters_ex(
+          ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "a", &servers)) {
+    if (NULL != servers && Z_TYPE_P(servers) == IS_ARRAY) {
+      ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(servers), server) {
+        zval* host = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 0);
+        zval* port = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 1);
+        if (nr_php_is_zval_valid_string(host) &&
+            nr_php_is_zval_valid_integer(port)) {
+          nr_php_memcached_create_instance_metric(Z_STRVAL_P(host), Z_LVAL_P(port));
+        }
+      }
+      ZEND_HASH_FOREACH_END();
+    }
+  }
+  zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler,
+                                     INTERNAL_FUNCTION_PARAM_PASSTHRU);
+
+  if (zcaught) {
+    zend_bailout();
+    /* NOTREACHED */
+  }
+}
+
 /*
  * Handle
  *   bool redis::connect ( string $host[, int $port = 6379 ... ] )
@@ -3098,6 +3151,8 @@ NR_OUTER_WRAPPER(memcached_set)
 NR_OUTER_WRAPPER(memcached_setbykey)
 NR_OUTER_WRAPPER(memcached_setmulti)
 NR_OUTER_WRAPPER(memcached_setmultibykey)
+NR_OUTER_WRAPPER(memcached_addserver)
+NR_OUTER_WRAPPER(memcached_addservers)
 
 NR_OUTER_WRAPPER(redis_append)
 NR_OUTER_WRAPPER(redis_bitcount)
@@ -3511,6 +3566,10 @@ void nr_php_generate_internal_wrap_records(void) {
                       memcache_function, 0, "set")
   NR_INTERNAL_WRAPREC("memcached::setmultibykey", memcached_setmultibykey,
                       memcache_function, 0, "set")
+  NR_INTERNAL_WRAPREC("memcached::addserver", memcached_addserver,
+                      memcached_add_server, 0, 0);
+  NR_INTERNAL_WRAPREC("memcached::addservers", memcached_addservers,
+                      memcached_add_servers, 0, 0);
 
   NR_INTERNAL_WRAPREC("redis::connect", redis_connect, redis_connect, 0,
                       "connect")

agent/php_memcached.c

@@ -0,0 +1,34 @@
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include "php_memcached.h"
+#include "nr_datastore_instance.h"
+#include "php_agent.h"
+
+nr_datastore_instance_t* nr_php_memcached_create_datastore_instance(
+    const char* host_or_socket,
+    zend_long port) {
+  nr_datastore_instance_t* instance = NULL;
+  if (port == 0) { // local socket
+    instance = nr_datastore_instance_create("localhost", host_or_socket, NULL);
+  } else {
+    char* port_str = nr_formatf("%ld", (long)port);
+    instance  = nr_datastore_instance_create(host_or_socket, port_str, NULL);
+    nr_free(port_str);
+  }
+  return instance;
+}
+
+void nr_php_memcached_create_instance_metric(
+    const char* host_or_socket,
+    zend_long port) {
+  nr_datastore_instance_t* instance
+    = nr_php_memcached_create_datastore_instance(host_or_socket, port);
+  char* instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s",
+                                     instance->host, instance->port_path_or_id);
+  nrm_force_add(NRPRG(txn)->unscoped_metrics, instance_metric, 0);
+  nr_datastore_instance_destroy(&instance);
+  nr_free(instance_metric);
+}

agent/php_memcached.h

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef PHP_MEMCACHED_HDR
+#define PHP_MEMCACHED_HDR
+
+#include "nr_datastore_instance.h"
+#include "php_includes.h"
+
+/*
+ * Purpose : Create a datastore instance metadata for a Memcached server.
+ *
+ * Params  : 1. The memcached host or socket name as given to Memcached::addServer().
+ *           2. The memcached port as given as given to Memcached::addServer().
+ *
+ * Returns: nr_datastore_instance_t* that the caller is responsible for freeing
+ */
+nr_datastore_instance_t* nr_php_memcached_create_datastore_instance(
+    const char* host_or_socket,
+    zend_long port);
+
+/*
+ * Purpose : Create a memcached instance metric
+ *
+ * Params  : 1. The memcached host or socket name as given to Memcached::addServer().
+ *              Must be non-null.
+ *           2. The memcached port as given as given to Memcached::addServer().
+ *              Must be non-null.
+ */
+extern void nr_php_memcached_create_instance_metric(
+    const char* host_or_socket,
+    zend_long port);
+
+
+#endif

agent/tests/test_memcached.c

@@ -0,0 +1,79 @@
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+#include "tlib_php.h"
+#include "tlib_datastore.h"
+
+#include "php_agent.h"
+#include "php_memcached.h"
+#include "util_system.h"
+
+tlib_parallel_info_t parallel_info
+    = {.suggested_nthreads = -1, .state_size = 0};
+
+static char* system_host_name;
+
+static void test_create_datastore_instance(void) {
+  assert_datastore_instance_equals_destroy(
+      "named socket",
+      &((nr_datastore_instance_t){
+          .host = system_host_name,
+          .database_name = "unknown",
+          .port_path_or_id = "/tmp/memcached.sock",
+      }),
+      nr_php_memcached_create_datastore_instance("/tmp/memcached.sock", 0));
+
+  assert_datastore_instance_equals_destroy(
+      "empty socket",
+      &((nr_datastore_instance_t){
+          .host = system_host_name,
+          .database_name = "unknown",
+          .port_path_or_id = "unknown",
+      }),
+      nr_php_memcached_create_datastore_instance("", 0));
+
+  assert_datastore_instance_equals_destroy(
+      "empty host",
+      &((nr_datastore_instance_t){
+          .host = system_host_name,
+          .database_name = "unknown",
+          .port_path_or_id = "unknown",
+      }),
+      nr_php_memcached_create_datastore_instance(NULL, 0));
+
+  assert_datastore_instance_equals_destroy(
+      "host.name socket",
+      &((nr_datastore_instance_t){
+          .host = "host.name",
+          .database_name = "unknown",
+          .port_path_or_id = "11211",
+      }),
+      nr_php_memcached_create_datastore_instance("host.name", 11211));
+
+  assert_datastore_instance_equals_destroy(
+      "host and port",
+      &((nr_datastore_instance_t){
+          .host = "unknown",
+          .database_name = "unknown",
+          .port_path_or_id = "6379",
+      }),
+      nr_php_memcached_create_datastore_instance("", 6379));
+
+  assert_datastore_instance_equals_destroy(
+      "NULL socket",
+      &((nr_datastore_instance_t){
+          .host = "unknown",
+          .database_name = "unknown",
+          .port_path_or_id = "11211",
+      }),
+      nr_php_memcached_create_datastore_instance(NULL, 11211));
+}
+
+void test_main(void* p NRUNUSED) {
+  system_host_name = nr_system_get_hostname();
+
+  test_create_datastore_instance();
+
+  nr_free(system_host_name);
+}

tests/integration/memcached/test_add_servers.php

@@ -0,0 +1,39 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+The agent should report instance metrics when multiple servers are
+added at once via Memcached::addServers()
+*/
+
+/*SKIPIF
+<?php require('skipif.inc'); ?>
+*/
+
+/*INI
+*/
+
+/*EXPECT_METRICS_EXIST
+Datastore/instance/Memcached/host1/1, 1
+Datastore/instance/Memcached/host2/2, 1
+Datastore/instance/Memcached/host3/11211, 1
+Datastore/instance/Memcached/host4/1, 1
+*/
+
+/*EXPECT_ERROR_EVENTS null */
+
+require_once(realpath (dirname ( __FILE__ )) . '/../../include/helpers.php');
+require_once(realpath (dirname ( __FILE__ )) . '/../../include/tap.php');
+require_once(realpath (dirname ( __FILE__ )) . '/memcache.inc');
+
+$memcached = new Memcached();
+$memcached->addServers(array(
+                       array("host1", 1),
+                       array("host2", 2),
+                       array("host3", 11211)));
+$memcached->addServers(array());
+$memcached->addServers(array(array("host4", 1, "test field")));
+$memcached->quit();

tests/integration/memcached/test_add_servers_bad.php

@@ -0,0 +1,68 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+The agent should gracefully handle when malformed server entries
+are added via Memcached::addServers()
+*/
+
+/*SKIPIF
+<?php require('skipif.inc'); ?>
+*/
+
+/*INI
+*/
+
+/*EXPECT_REGEX
+
+.*(PHP )?Warning:.*could not add entry.*
+
+.*(PHP )?Warning:.*could not add entry.*
+
+*/
+
+/*EXPECT_ERROR_EVENTS
+[
+  "?? agent run id",
+  {
+    "reservoir_size": "??",
+    "events_seen": 1
+  },
+  [
+    [
+      {
+        "type": "TransactionError",
+        "timestamp": "??",
+        "error.class": "E_WARNING",
+        "error.message": "Memcached::addServers(): could not add entry #2 to the server list",
+        "transactionName": "OtherTransaction\/php__FILE__",
+        "duration": "??",
+        "nr.transactionGuid": "??",
+        "guid": "??",
+        "sampled": true,
+        "priority": "??",
+        "traceId": "??",
+        "spanId": "??"
+      },
+      {},
+      {}
+    ]
+  ]
+]
+*/
+
+require_once(realpath (dirname ( __FILE__ )) . '/../../include/helpers.php');
+require_once(realpath (dirname ( __FILE__ )) . '/../../include/tap.php');
+require_once(realpath (dirname ( __FILE__ )) . '/memcache.inc');
+
+$memcached = new Memcached();
+$memcached->addServer(5, 5);
+//$memcached->addServer("host", string); crashes PHP
+$memcached->addServers(array(array(1)));
+$memcached->addServers(array(array("host1")));
+$memcached->addServers(array(array(1, "host1")));
+//$memcahed->addServers("string"); crashes PHP
+$memcached->quit();

tests/integration/memcached/test_basic.php

@@ -46,6 +46,7 @@
     [{"name":"Datastore/allOther"},                                   [15, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/Memcached/all"},                              [15, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/Memcached/allOther"},                         [15, "??", "??", "??", "??", "??"]],
+    [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/operation/Memcached/add"},                    [2, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/operation/Memcached/add",
       "scope":"OtherTransaction/php__FILE__"},                        [2, "??", "??", "??", "??", "??"]],

tests/integration/memcached/test_basic_logging_off.php

@@ -49,6 +49,7 @@
     [{"name":"Datastore/allOther"},                                   [15, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/Memcached/all"},                              [15, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/Memcached/allOther"},                         [15, "??", "??", "??", "??", "??"]],
+    [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/operation/Memcached/add"},                    [2, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/operation/Memcached/add",
       "scope":"OtherTransaction/php__FILE__"},                        [2, "??", "??", "??", "??", "??"]],

tests/integration/memcached/test_by_key.php

@@ -48,6 +48,7 @@
     [{"name":"Datastore/allOther"},                                   [11, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/Memcached/all"},                              [11, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/Memcached/allOther"},                         [11, "??", "??", "??", "??", "??"]],
+    [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/operation/Memcached/add"},                    [2, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/operation/Memcached/add",
       "scope":"OtherTransaction/php__FILE__"},                        [2, "??", "??", "??", "??", "??"]],

tests/integration/memcached/test_cas.php7.php

@@ -42,6 +42,7 @@
     [{"name":"Datastore/allOther"},                                   [5, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/Memcached/all"},                              [5, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/Memcached/allOther"},                         [5, "??", "??", "??", "??", "??"]],
+    [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/operation/Memcached/delete"},                 [1, "??", "??", "??", "??", "??"]],
     [{"name":"Datastore/operation/Memcached/delete",
       "scope":"OtherTransaction/php__FILE__"},                        [1, "??", "??", "??", "??", "??"]],