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

feat(agent): Memcached instance metrics with host name #958

Merged
merged 52 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
275f841
create named datastore metrics for memcached
ZNeumann Aug 23, 2024
a462c20
restrict metrics
ZNeumann Aug 26, 2024
b5cef15
fixup
ZNeumann Aug 28, 2024
6530778
integration tests
ZNeumann Aug 28, 2024
4c46e86
fix expecteds
ZNeumann Aug 28, 2024
ca82d09
refactor
ZNeumann Sep 4, 2024
b777c76
PR feedback
ZNeumann Sep 17, 2024
31186d8
add memcached instance socket test
ZNeumann Sep 20, 2024
9fb7bd3
doc string
ZNeumann Sep 24, 2024
55af999
Update agent/php_memcached.c
ZNeumann Sep 24, 2024
eb038da
host_or_socket name
ZNeumann Sep 25, 2024
f7878e0
Update agent/php_internal_instrument.c
ZNeumann Sep 26, 2024
db755da
Update agent/php_internal_instrument.c
ZNeumann Sep 26, 2024
ee6ad5f
Update agent/php_internal_instrument.c
ZNeumann Sep 26, 2024
330e976
doc string
ZNeumann Sep 26, 2024
c60dacd
move datastore op call
ZNeumann Sep 26, 2024
b38b9ca
return status
ZNeumann Sep 26, 2024
1b8a849
add empty addservers test
ZNeumann Sep 26, 2024
5d165f2
fixups
ZNeumann Sep 26, 2024
92ac8d5
unit test
ZNeumann Sep 26, 2024
007c774
more unit tests
ZNeumann Sep 26, 2024
1e97b44
refactor lines
ZNeumann Sep 26, 2024
6db55f6
NULL unit test
ZNeumann Sep 26, 2024
f447a56
rename add_server
ZNeumann Sep 26, 2024
ae2a70b
NULL check
ZNeumann Sep 26, 2024
cf1f772
fix
ZNeumann Sep 26, 2024
330b5c6
more tests
ZNeumann Sep 27, 2024
c8b0880
simplify everything
ZNeumann Sep 27, 2024
8c07bd8
test comment
ZNeumann Sep 27, 2024
5ca4df6
remove old code
ZNeumann Sep 27, 2024
3869a62
split tests
ZNeumann Sep 27, 2024
9fae58d
unscoped metrics
ZNeumann Sep 27, 2024
a78bcf4
modify socket test
ZNeumann Sep 27, 2024
2125e6f
remove accident
ZNeumann Sep 27, 2024
368a2ba
remove ZTS block
ZNeumann Sep 27, 2024
06cf165
fix regex
ZNeumann Sep 27, 2024
18fff7a
free better
ZNeumann Oct 1, 2024
c40cbd4
fix function name
ZNeumann Oct 1, 2024
68843dc
update makefile
ZNeumann Oct 3, 2024
63beced
PR comments
ZNeumann Oct 3, 2024
5597d7f
concise unit tests
ZNeumann Oct 3, 2024
7cbf7f4
more integration tests
ZNeumann Oct 3, 2024
65424c6
environment variables in expecteds
ZNeumann Oct 3, 2024
8ec0268
socket path
ZNeumann Oct 3, 2024
a40b011
remove extern
ZNeumann Oct 3, 2024
0453459
revert port
ZNeumann Oct 3, 2024
62b8889
remove comment
ZNeumann Oct 3, 2024
1b227d3
reword
ZNeumann Oct 3, 2024
90f1151
Update agent/php_internal_instrument.c
ZNeumann Oct 3, 2024
3364eb0
fix comment
ZNeumann Oct 3, 2024
c8bdfb7
remove incorrect comment
ZNeumann Oct 3, 2024
6e720c9
more tests
ZNeumann Oct 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions agent/Makefile.frag
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
2 changes: 1 addition & 1 deletion agent/config.m4
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
59 changes: 59 additions & 0 deletions agent/php_internal_instrument.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
zsistla marked this conversation as resolved.
Show resolved Hide resolved
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 ... ] )
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down
34 changes: 34 additions & 0 deletions agent/php_memcached.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright 2020 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

#include "php_memcached.h"
ZNeumann marked this conversation as resolved.
Show resolved Hide resolved
#include "nr_datastore_instance.h"
#include "php_agent.h"

nr_datastore_instance_t* nr_php_memcached_create_datastore_instance(
lavarou marked this conversation as resolved.
Show resolved Hide resolved
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);
lavarou marked this conversation as resolved.
Show resolved Hide resolved
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);
}
35 changes: 35 additions & 0 deletions agent/php_memcached.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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"

lavarou marked this conversation as resolved.
Show resolved Hide resolved
/*
* 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().
lavarou marked this conversation as resolved.
Show resolved Hide resolved
*
* 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().
* 2. The memcached port as given as given to Memcached::addServer().
*/
extern void nr_php_memcached_create_instance_metric(
const char* host_or_socket,
zend_long port);


#endif
127 changes: 127 additions & 0 deletions agent/tests/test_memcached.c
lavarou marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/*
* 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));
}

static void test_create_instance_metric(void) {
nrtxn_t* txn;
nrmetric_t* metric;
char* metric_str;
tlib_php_engine_create("");
tlib_php_request_start();
txn = NRPRG(txn);

nr_php_memcached_create_instance_metric("host", 11211);
metric = nrm_find(txn->unscoped_metrics, "Datastore/instance/Memcached/host/11211");
tlib_pass_if_not_null("metric found", metric);

nr_php_memcached_create_instance_metric("", 11211);
metric = nrm_find(txn->unscoped_metrics, "Datastore/instance/Memcached/unknown/11211");
tlib_pass_if_not_null("metric found", metric);

nr_php_memcached_create_instance_metric(NULL, 7);
metric = nrm_find(txn->unscoped_metrics, "Datastore/instance/Memcached/unknown/7");
tlib_pass_if_not_null("metric found", metric);

nr_php_memcached_create_instance_metric("path/to/sock", 0);
metric_str = nr_formatf("Datastore/instance/Memcached/%s/path/to/sock", system_host_name);
metric = nrm_find(txn->unscoped_metrics, metric_str);
nr_free(metric_str);
tlib_pass_if_not_null("metric found", metric);

nr_php_memcached_create_instance_metric("", 0);
metric_str = nr_formatf("Datastore/instance/Memcached/%s/unknown", system_host_name);
metric = nrm_find(txn->unscoped_metrics, metric_str);
nr_free(metric_str);
tlib_pass_if_not_null("metric found", metric);

// restart the transaction because the next metric is the same as a previous metric
tlib_php_request_end();
tlib_php_request_start();
txn = NRPRG(txn);

nr_php_memcached_create_instance_metric(NULL, 0);
metric_str = nr_formatf("Datastore/instance/Memcached/%s/unknown", system_host_name);
metric = nrm_find(txn->unscoped_metrics, metric_str);
nr_free(metric_str);
tlib_pass_if_not_null("metric found", metric);

tlib_php_request_end();
tlib_php_engine_destroy();
}

void test_main(void* p NRUNUSED) {
system_host_name = nr_system_get_hostname();

test_create_datastore_instance();
test_create_instance_metric();

nr_free(system_host_name);
}
39 changes: 39 additions & 0 deletions tests/integration/memcached/test_add_servers.php
Original file line number Diff line number Diff line change
@@ -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
*/
lavarou marked this conversation as resolved.
Show resolved Hide resolved

/*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(
bduranleau-nr marked this conversation as resolved.
Show resolved Hide resolved
array("host1", 1),
array("host2", 2),
array("host3", 11211)));
$memcached->addServers(array());
lavarou marked this conversation as resolved.
Show resolved Hide resolved
$memcached->addServers(array(array("host4", 1, "test field")));
$memcached->quit();
Loading