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): Adds label forwarding to log events #1027

Open
wants to merge 60 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
012793d
feat(agent): Adds INI values for labels for forwarded logs
mfulb Feb 11, 2025
6c84bb9
feat(agent): Adds processing of log label from labels
mfulb Feb 11, 2025
f38315d
feat(agent): Label forwarding for APM logs
mfulb Feb 13, 2025
78c8388
feat(agent): Adds log forwarding labels supportability metric
mfulb Feb 18, 2025
520169e
tests(logging): Adds Monolog3 tests for log forwarding labels
mfulb Feb 18, 2025
9a3cd24
chore(tests): Removes unneeded spaces
mfulb Feb 18, 2025
5983bc0
chore(daemon): Remove debugging statement
mfulb Feb 19, 2025
b8c415a
tests(daemon): Add simple tests for LogEvents
mfulb Feb 19, 2025
ed8cada
chore(daemon): Removes unneeded import
mfulb Feb 19, 2025
6c0b65f
chore(daemon): Logs unmarshall error
mfulb Feb 19, 2025
0672ac0
tests(daemon): Adds log forwarded label to test
mfulb Feb 19, 2025
5b23982
fix(agent): Fixes log forwarded labels definition
mfulb Feb 19, 2025
8186043
fix(agent): Fixes encoding of forwarded log labels to an Event
mfulb Feb 19, 2025
993403b
tests(agent): Adds test for encoding forwarded log labels
mfulb Feb 19, 2025
b5616d0
fix(agent): Updates function call for nr_txn_begin()
mfulb Feb 19, 2025
bea9a98
fix(agent): Fixes decoding of forwarded log labels as an Event
mfulb Feb 19, 2025
7c9d436
fix(agent): Fixes log_forwarding_labels definition
mfulb Feb 19, 2025
b476cda
chore(daemon): Restores original import order
mfulb Feb 19, 2025
7545636
chore(daemon): Fixes formatting
mfulb Feb 19, 2025
5b906ef
tests(agent): Adds log label forwarding tests for Monolog2
mfulb Feb 19, 2025
0b1b0b5
tests(agent): Updates monolog (2 & 3) tests to include new log forwar…
mfulb Feb 20, 2025
2d0e4ad
chore(docs): Corrects comment for log forwarded labels exclude rule
mfulb Feb 20, 2025
2c53c7a
chore(docs): Fixes version for when php_packages added to protobuf
mfulb Feb 20, 2025
c0a7666
chore: Removes test_segment_message - moving to another PR
mfulb Feb 20, 2025
5426df7
tests(daemon): Tests invalid data passed to SetLogForwardingLabels
mfulb Feb 20, 2025
eb8b575
tests(axiom): Adds test if log fowarded labels is NULL
mfulb Feb 20, 2025
ddf40e0
tests(agent): Updates integration/api tests to have log labels metric
mfulb Feb 24, 2025
c7fdf4a
tests(agent): Updates integration/basic tests to have log labels metric
mfulb Feb 24, 2025
f921ddb
tests(agent): Updates integration/distributed_tracing tests to have l…
mfulb Feb 24, 2025
f73cfa0
tests(agent): Updates integration/errors tests to have log labels metric
mfulb Feb 24, 2025
b5e7121
tests(agent): Updates integration/external tests to have log labels m…
mfulb Feb 24, 2025
83b2cbf
tests(agent): Updates integration/api tests to have log labels metric
mfulb Feb 24, 2025
1ed7d03
tests(agent): Updates integration/frameworks tests to have log labels…
mfulb Feb 24, 2025
c6faa75
tests(agent): Updates integration/ini tests to have log labels metric
mfulb Feb 24, 2025
5ef7858
tests(agent): Updates integration/lang tests to have log labels metric
mfulb Feb 24, 2025
a7595b0
tests(agent): Updates integration/memcache tests to have log labels m…
mfulb Feb 24, 2025
da69a7c
tests(agent): Updates integration/memcached tests to have log labels …
mfulb Feb 24, 2025
f02d0da
tests(agent): Updates integration/mysql tests to have log labels metric
mfulb Feb 24, 2025
a3bad9b
tests(agent): Updates integration/mysqli tests to have log labels metric
mfulb Feb 24, 2025
b97fa42
tests(agent): Updates integration/pdo tests to have log labels metric
mfulb Feb 24, 2025
95f0d6c
tests(agent): Updates integration/pgsql tests to have log labels metric
mfulb Feb 24, 2025
1e8c326
tests(agent): Updates integration/predis tests to have log labels metric
mfulb Feb 24, 2025
3b40481
tests(agent): Updates integration/queue tests to have log labels metric
mfulb Feb 24, 2025
3167869
tests(agent): Updates integration/redis tests to have log labels metric
mfulb Feb 24, 2025
2822bab
tests(agent): Updates integration/sqlite* tests to have log labels me…
mfulb Feb 24, 2025
ee16060
tests(agent): Adds another test for log forwarded labels using defaults
mfulb Feb 25, 2025
c9bea37
fix(agent): Restores gitignore line for test_segment_message
mfulb Feb 27, 2025
192e04e
Update agent/scripts/newrelic.ini.template
mfulb Feb 27, 2025
277d034
chore(agent): Improves INI description for log labels exclude directive
mfulb Feb 27, 2025
467c8a2
Update protocol/flatbuffers/protocol.fbs
mfulb Mar 6, 2025
ee9a5a4
tests(daemon): Improves log labels ingest unit tests
mfulb Mar 6, 2025
8d46a55
fix(daemon): Verify log labels are correct format
mfulb Mar 6, 2025
7a02270
chore(docs): Improves api doc for nr_txn_begin()
mfulb Mar 6, 2025
c13d6c0
fix(agent): Do not all log labels with a NULL key
mfulb Mar 6, 2025
a238c67
fix(agent): Protects against possiblity key is NULL
mfulb Mar 6, 2025
90955f0
chore(agent): Refactors log label exclusion logic
mfulb Mar 6, 2025
67c5348
tests(agent): Adds check for log forwarding labels config
mfulb Mar 6, 2025
a3aaad0
tests(agent): Adds unit tests for log label creation
mfulb Mar 6, 2025
47df6f2
chore: Fix formatting
mfulb Mar 6, 2025
65f9ba2
tests(agent): Adds some more test cases for log labels
mfulb Mar 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
6 changes: 6 additions & 0 deletions agent/php_newrelic.h
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,12 @@ nrinibool_t
nriniuint_t
log_forwarding_log_level; /* newrelic.application_logging.forwarding.log_level
*/
nrinibool_t
log_forwarding_labels_enabled; /* newrelic.application_logging.forwarding.labels.enabled */

nrinistr_t
log_forwarding_labels_exclude; /* newrelic.application_logging.forwarding.labels.exclude */


/*
* Configuration option to toggle code level metrics collection.
Expand Down
16 changes: 16 additions & 0 deletions agent/php_nrini.c
Original file line number Diff line number Diff line change
Expand Up @@ -3078,6 +3078,22 @@ STD_PHP_INI_ENTRY_EX("newrelic.application_logging.forwarding.context_data.exclu
zend_newrelic_globals,
newrelic_globals,
0)
STD_PHP_INI_ENTRY_EX("newrelic.application_logging.forwarding.labels.enabled",
"0",
NR_PHP_REQUEST,
nr_boolean_mh,
log_forwarding_labels_enabled,
zend_newrelic_globals,
newrelic_globals,
nr_enabled_disabled_dh)
STD_PHP_INI_ENTRY_EX("newrelic.application_logging.forwarding.labels.exclude",
"",
NR_PHP_REQUEST,
nr_string_mh,
log_forwarding_labels_exclude,
zend_newrelic_globals,
newrelic_globals,
0)

/*
* Vulnerability Management
Expand Down
128 changes: 126 additions & 2 deletions agent/php_txn.c
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,119 @@ static nrobj_t* nr_php_txn_get_labels() {
return nr_labels_parse(NR_PHP_PROCESS_GLOBALS(env_labels));
}

static nr_status_t nr_php_txn_collect_label_keys_iter(const char* key,
const nrobj_t* value,
void* ptr) {
nrobj_t* user_data = (nrobj_t*)ptr;

(void)value;

if (NULL == key || NULL == user_data) {
return NR_FAILURE;
}

nro_set_array_string(user_data, 0, key);

return NR_SUCCESS;
}

/*
* Purpose : Filter the labels hash to exclude any labels that are in the
* newrelic.application_logging.forwarding.labels.exclude list.
*
* Params : 1. The labels hash to filter.
*
* Returns : A new hash containing the filtered labels.
* If no labels exist or all labels are excluded, then return NULL.
*
*/

nrobj_t* nr_php_txn_get_log_forwarding_labels(nrobj_t* labels) {
nrobj_t* label_keys = NULL;
nrobj_t* exclude_labels_list = NULL;
nrobj_t* exclude_labels_hash = NULL;
nrobj_t* log_labels = NULL;

if (NULL == labels || 0 == nro_getsize(labels)) {
nrl_verbosedebug(NRL_TXN, "%s: No labels defined", __FUNCTION__);
return NULL;
}

/* if logging labels are disabled then nothing to do */
if (0 == NRINI(log_forwarding_labels_enabled)) {
nrl_verbosedebug(NRL_TXN, "%s: Log forwarding labels disabled",
__FUNCTION__);
return NULL;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

A check if log_forwarding_labels_exclude is an empty string (most common case) would allow an early exit from the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the exclude rule is empty we need to copy all the labels into the log labels structure so I don't see how we could exit early.

Copy link
Contributor

@zsistla zsistla Mar 7, 2025

Choose a reason for hiding this comment

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

Why do we need to copy all the log labels if there are no excludes?
We know that we want to include everything, so if the exclude list is the an empty string, couldn't we just could exit early here and return labels?

/* split exclude string on commas - nr_strsplit() will trim leading
* and trailing whitespace from each string extracted, as well as
* ignoring empty strings after whitespace trimming
*/
exclude_labels_list
= nr_strsplit(NRINI(log_forwarding_labels_exclude), ",", 0);

/* convert to lowercase to support case insensitive search below
* will store lowercase version in a hash for more convenient lookup
*/
exclude_labels_hash = nro_new(NR_OBJECT_HASH);
for (int i = 0; i < nro_getsize(exclude_labels_list); i++) {
char* label = nr_string_to_lowercase(
nro_get_array_string(exclude_labels_list, i + 1, NULL));

if (!nr_strempty(label)) {
nro_set_hash_boolean(exclude_labels_hash, label, 1);
}
nr_free(label);
}

/* original parsed exclude list is no longer needed */
nro_delete(exclude_labels_list);

/* collect label keys from existing labels */
label_keys = nro_new(NR_OBJECT_ARRAY);
nro_iteratehash(labels, nr_php_txn_collect_label_keys_iter,
(void*)label_keys);

/* filter by going over the list of label keys, seeing if it exists in the
* exclude hash, and if it does skip it otherwise copy key/value for label
* to the log labels
*/
log_labels = nro_new(NR_OBJECT_HASH);
for (int i = 0; i < nro_getsize(label_keys); i++) {
const char* key = NULL;
char* lower_key = NULL;
int exclude = false;

key = nro_get_array_string(label_keys, i + 1, NULL);
if (NULL == key) {
continue;
}

lower_key = nr_string_to_lowercase(key);

if (1 != nro_get_hash_boolean(exclude_labels_hash, lower_key, NULL)) {
nro_set_hash_string(log_labels, key,
nro_get_hash_string(labels, key, NULL));
} else {
nrl_verbosedebug(NRL_TXN, "%s: Excluding label %s", __FUNCTION__,
NRSAFESTR(key));
}
nr_free(lower_key);
}

nro_delete(exclude_labels_hash);
nro_delete(label_keys);

/* return NULL if all labels were excluded */
if (0 == nro_getsize(log_labels)) {
nro_delete(log_labels);
log_labels = NULL;
}

return log_labels;
}

static void nr_php_txn_prepared_statement_destroy(void* sql) {
nr_free(sql);
}
Expand Down Expand Up @@ -666,6 +779,11 @@ static void nr_php_txn_send_metrics_once(nrtxn_t* txn TSRMLS_DC) {
nrm_force_add(NRTXN(unscoped_metrics), metname, 0);
nr_free(metname);

metname = nr_formatf("Supportability/Logging/Labels/PHP/%s",
FMT_BOOL(nr_txn_log_forwarding_labels_enabled(txn)));
nrm_force_add(NRTXN(unscoped_metrics), metname, 0);
nr_free(metname);

txn->created_logging_onetime_metrics = true;

#undef FMT_BOOL
Expand Down Expand Up @@ -762,6 +880,7 @@ nr_status_t nr_php_txn_begin(const char* appnames,
nrtxnopt_t opts;
const char* lic_to_use;
int pfd;
nrobj_t* log_forwarding_labels = NULL;
nr_attribute_config_t* attribute_config;
nr_app_info_t info;
bool is_cli = (0 != NR_PHP_PROCESS_GLOBALS(cli));
Expand Down Expand Up @@ -854,6 +973,7 @@ nr_status_t nr_php_txn_begin(const char* appnames,
opts.log_forwarding_log_level = NRINI(log_forwarding_log_level);
opts.log_events_max_samples_stored = NRINI(log_events_max_samples_stored);
opts.log_metrics_enabled = NRINI(log_metrics_enabled);
opts.log_forwarding_labels_enabled = NRINI(log_forwarding_labels_enabled);
opts.message_tracer_segment_parameters_enabled
= NRINI(message_tracer_segment_parameters_enabled);

Expand Down Expand Up @@ -917,17 +1037,21 @@ nr_status_t nr_php_txn_begin(const char* appnames,
&nr_php_app_settings, NR_PHP_PROCESS_GLOBALS(daemon_app_connect_timeout));
nr_app_info_destroy_fields(&info);

if (0 == NRPRG(app)) {
if (NULL == NRPRG(app)) {
nrl_debug(NRL_INIT, "unable to begin transaction: app '%.128s' is unknown",
appnames ? appnames : "");
return NR_FAILURE;
}

attribute_config = nr_php_create_attribute_config(TSRMLS_C);
NRPRG(txn) = nr_txn_begin(NRPRG(app), &opts, attribute_config);
log_forwarding_labels
= nr_php_txn_get_log_forwarding_labels(NRPRG(app)->info.labels);
NRPRG(txn) = nr_txn_begin(NRPRG(app), &opts, attribute_config,
log_forwarding_labels);
nrt_mutex_unlock(&(NRPRG(app)->app_lock));

nr_attribute_config_destroy(&attribute_config);
nro_delete(log_forwarding_labels);

if (0 == NRPRG(txn)) {
nrl_debug(NRL_INIT, "no Axiom transaction this time around");
Expand Down
13 changes: 13 additions & 0 deletions agent/php_txn_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,16 @@ extern void nr_php_txn_php_package_create_major_metric(void* value,
* Params : 1. The current transaction.
*/
extern void nr_php_txn_create_packages_major_metrics(nrtxn_t* txn);

/*
* Purpose : Filter the labels hash to exclude any labels that are in the
* newrelic.application_logging.forwarding.labels.exclude list.
*
* Params : 1. The labels hash to filter.
*
* Returns : A new hash containing the filtered labels.
* If no labels exist or all labels are excluded, then return NULL.
*
*/

extern nrobj_t* nr_php_txn_get_log_forwarding_labels(nrobj_t* labels);
29 changes: 28 additions & 1 deletion agent/scripts/newrelic.ini.template
Original file line number Diff line number Diff line change
Expand Up @@ -1242,7 +1242,6 @@ newrelic.daemon.logfile = "/var/log/newrelic/newrelic-daemon.log"
;
;newrelic.application_logging.forwarding.log_level = "WARNING"


; Setting: newrelic.application_logging.local_decorating.enabled
; Type : boolean
; Scope : per-directory
Expand Down Expand Up @@ -1305,6 +1304,34 @@ newrelic.daemon.logfile = "/var/log/newrelic/newrelic-daemon.log"
;newrelic.application_logging.forwarding.context_data.include = ""
;newrelic.application_logging.forwarding.context_data.exclude = ""

; Setting: newrelic.application_logging.forwarding.labels.enabled
; Type : boolean
; Scope : per-directory
; Default: false
; Info : Toggles whether the agent adds labels as attributes to log records which
; are sent to New Relic. The labels can be defined by the newrelic.labels
; configuration value, and filtered using the
; newrelic.application_logging.forwarding.labels.exclude configuration value.
;
;newrelic.application_logging.forwarding.labels.enabled = false

; Setting: newrelic.application_logging.forwarding.labels.exclude
; Type : string
; Scope : per-directory
; Default: ""
; Info : A list of labels to NOT add as attributes to logs which are forwarded
; to New Relic. The values in the list must be separated by commas.
;
; NOTE: The values in this list are compared to label key names in a case
; insensitive fashion. So if an exclude rule for "server" is specified
; here then it would exclude keys like "server", "Server", "SERVER", and
; any other combination of lower and upper case letters that spell out "server".
;
; Ex:
; newrelic.application_logging.forwarding.labels.exclude = "label1, label2"
;
;newrelic.application_logging.forwarding.labels.exclude = ""

; Setting: newrelic.code_level_metrics.enabled
; Type : boolean
; Scope : per-directory
Expand Down
69 changes: 67 additions & 2 deletions agent/tests/test_txn.c
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,69 @@ static void test_create_agent_php_version_metrics() {
#undef PHP_VERSION_METRIC_BASE
#undef AGENT_VERSION_METRIC_BASE

static void test_create_log_forwarding_labels(TSRMLS_D) {
nrobj_t* labels = NULL;
nrobj_t* log_labels = NULL;
char* json = NULL;

/* Test : Create log forwarding labels with valid key/value pairs */
labels = nro_new_hash();
nro_set_hash_string(labels, "key1", "value1");
nro_set_hash_string(labels, "key2", "value2");
nro_set_hash_string(labels, "key3", "value3");

log_labels = nr_php_txn_get_log_forwarding_labels(labels);

json = nro_to_json(labels);
tlib_pass_if_str_equal(
"valid log label creation test",
"{\"key1\":\"value1\",\"key2\":\"value2\",\"key3\":\"value3\"}", json);

nr_free(json);
nro_delete(labels);
nro_delete(log_labels);

/* Test : Create log forwarding labels with empty key/value pairs */
labels = nro_new_hash();
nro_set_hash_string(labels, "", "");
nro_set_hash_string(labels, "key", "");
nro_set_hash_string(labels, "", "value");

log_labels = nr_php_txn_get_log_forwarding_labels(labels);

json = nro_to_json(labels);
tlib_pass_if_str_equal("empty string log label creation test",
"{\"key\":\"\"}", json);

nr_free(json);
nro_delete(labels);
nro_delete(log_labels);

/* Test : Create log forwarding labels with NULL key/value pairs */
labels = nro_new_hash();
nro_set_hash_string(labels, NULL, NULL);
nro_set_hash_string(labels, "key", NULL);
nro_set_hash_string(labels, NULL, "value");

log_labels = nr_php_txn_get_log_forwarding_labels(labels);

json = nro_to_json(labels);
tlib_pass_if_str_equal("NULL value log label creation test", "{\"key\":\"\"}",
json);

nr_free(json);
nro_delete(labels);
nro_delete(log_labels);

/* Test : Create log forwarding labels NULL labels object */
log_labels = nr_php_txn_get_log_forwarding_labels(NULL);
json = nro_to_json(labels);
tlib_pass_if_str_equal("NULL object log label creation test", "null", json);

nr_free(json);
nro_delete(log_labels);
}

tlib_parallel_info_t parallel_info = {.suggested_nthreads = 1, .state_size = 0};

void test_main(void* p NRUNUSED) {
Expand All @@ -299,14 +362,16 @@ void test_main(void* p NRUNUSED) {
* We're setting up our own engine instance because we need to control the
* attribute configuration.
*/
tlib_php_engine_create(
"newrelic.transaction_events.attributes.include=request.uri" PTSRMLS_CC);
// clang-format off
tlib_php_engine_create("newrelic.transaction_events.attributes.include=request.uri" PTSRMLS_CC);
// clang-format on

test_handle_fpm_error();
test_max_segments_config_values();
test_create_php_version_metric();
test_create_agent_version_metric();
test_create_agent_php_version_metrics();
test_create_log_forwarding_labels();

tlib_php_engine_destroy();
}
Loading