telemetry

[Dhiren-codity]

Description

Please include a summary of the change and which issue is fixed. List any dependencies that are required for this change.

Fixes #(issue_no)

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• Documentation update

Checklist:

• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• I have added tests/screenshots (if any) that prove my fix is effective or that my feature works.
• I have tested the tests implicated (if any) by my own code and they pass (make test or ctest -VV -R <test-name>).
• If my change is significant or breaking, I have passed all tests with ./docker-compose.sh test &> output and attached the output.
• I have tested my code with OPTION_BUILD_ADDRESS_SANITIZER or ./docker-compose.sh test-address-sanitizer &> output and OPTION_TEST_MEMORYCHECK.
• I have tested my code with OPTION_BUILD_THREAD_SANITIZER or ./docker-compose.sh test-thread-sanitizer &> output.
• I have tested with Helgrind in case my code works with threading.
• I have run make clang-format in order to format my code and my code follows the style guidelines.

If you are unclear about any of the above checks, have a look at our documentation here.

[Dhiren-codity]

/codity review

[Dhiren-codity]

@codity review

[codity-dev]

🔍 PR review started! Estimated time: 20-25 minutes.

💡 Learn More

Ask Codity questions:
Mention @codity {your question} in a comment to get answers about the code.

Trigger a manual review:
Comment @codity review on a PR or MR.

View Full Docs

[codity-dev]

🔍 PR review is completed!

[Dhiren-codity]

@codity review

[codity-dev]

🔍 PR review started! Estimated time: 20-25 minutes.

💡 Learn More

Ask Codity questions:
Mention @codity {your question} in a comment to get answers about the code.

Trigger a manual review:
Comment @codity review on a PR or MR.

View Full Docs

[codity-dev]

PR Summary

• The PR introduces a comprehensive telemetry library for the MetaCall polyglot runtime, enhancing logging and metrics capabilities across multiple programming languages (C, Python, JavaScript, and Java).

• Key Additions:

  • New classes and methods for structured logging, including Logger, LogHandler, Counter, Gauge, Histogram, and MetricRegistry.
  • Enums for log levels and metric export formats, improving configurability and extensibility.
  • A detailed README.md file outlining features and example usage.
  • New decorators in Python for automatic metric tracking.
  • JavaScript bindings for telemetry functionalities, improving usability in JavaScript applications.

• Walkthrough of Changes:

  • New Classes: Introduced classes for logging and metrics management, including scoped logging with CategoryLogger in Java.
  • Logging Methods: Added multiple logging methods for different severity levels, enhancing structured logging.
  • Metric Classes: Created classes for various metric types (Counter, Gauge, Histogram) to facilitate detailed telemetry data collection.
  • Configuration Headers: New header files (telemetry_api.h, telemetry_config.h, telemetry_metrics.h, telemetry_log.h) define the API and configuration settings for the telemetry library.
  • Functionality Enhancements: Added functions for metric management, exporting metrics in various formats, and managing log filters.
  • Testing Framework: Comprehensive test suites for logging and metrics systems ensure reliability and correctness of functionalities.

• Files with Code Changes:

  • telemetry_api.h: Defines the API for the telemetry library.
  • telemetry_config.h: Introduces configuration structures for logging and metrics.
  • telemetry_log.h: Contains logging-related structures and functions.
  • telemetry_metrics.h: Declares functions for managing telemetry metrics.
  • telemetry_metrics.c: Implements telemetry metrics management and logging functions.
  • test_log.c: Validates the functionality of the logging system.
  • test_metrics.c: Tests the telemetry metrics system.
  • JavaScript telemetry file: Implements telemetry library interface for JavaScript.

• Impact:

  • The changes significantly enhance the telemetry and logging capabilities of the MetaCall framework, improving observability, performance monitoring, and maintainability of the codebase. The structured approach to logging and metrics management facilitates better debugging and performance optimization.

[codity-dev]

Function 'portability' imported from 'portability/portability.h' but definition not found

Recommendation: Ensure the function portability is properly defined in the expected module, or remove the unused import to prevent runtime errors.

Additional Details

Details:

• imported_name: portability
• source_module: portability/portability.h
• import_type: direct_import

[codity-dev]

The increment method uses value.get() twice in the compareAndSet loop, which can lead to incorrect behavior if the value changes between the two calls.

Code Suggestion or Comments

public void increment(double delta) {
        double newValue;
        double current;
        do {
            current = value.get();
            newValue = current + delta;
        } while (!value.compareAndSet(current, newValue));
        updateMinMax(newValue);
    }

[codity-dev]

The test functions are well-structured, but adding comments to describe the purpose of each test case would improve readability and maintainability.

Code Suggestion or Comments

/* Test JSON export format */
void test_json_export(void) {
    // This test verifies that metrics can be exported in JSON format correctly
    printf("Testing JSON export format...\n");
    ...
}

[codity-dev]

The test descriptions could be more descriptive to provide better context on what each test is verifying.

Code Suggestion or Comments

test('metric exporter should register and unregister correctly', () => {
	MetricRegistry.initialize();

	const exporter = new telemetry.MetricExporter(
		MetricExportFormat.PROMETHEUS,
		"/tmp/metrics.txt",
		60
	);

	exporter.register();
	exporter.unregister();

	MetricRegistry.shutdown();
});

// Test histogram buckets
test('histogram buckets should correctly observe values', () => {
	MetricRegistry.initialize();

	const histogram = new Histogram("bucket_test", "Test buckets", "ms");
	histogram.observe(0.1);
	histogram.observe(1.5);
	histogram.observe(8.0);

	const buckets = histogram.getBuckets();
	assert(buckets.length > 0, "Should have buckets");

	MetricRegistry.unregister(histogram);
	MetricRegistry.shutdown();
});

// Test gauge min/max tracking

[codity-dev]

The observe method does not handle the case where the value is greater than all bucket upper bounds, which could lead to incorrect bucket counts.

Code Suggestion or Comments

boolean bucketUpdated = false;
        for (int i = 0; i < buckets.length; i++) {
            if (value <= buckets[i]) {
                bucketCounts[i].incrementAndGet();
                bucketUpdated = true;
                break;
            }
        }
        if (!bucketUpdated) {
            // Handle the case where the value is greater than all bucket upper bounds
            // This could be logging or incrementing a special overflow bucket
        }

[codity-dev]

The reset method does not reset the min and max values correctly if the histogram is reused, which could lead to incorrect statistics.

Code Suggestion or Comments

public void reset() {
        // TODO: Call native C library function
        totalCount.set(0);
        sum.set(0.0);
        min = Double.MAX_VALUE;
        max = -Double.MAX_VALUE;
        for (AtomicLong count : bucketCounts) {
            count.set(0);
        }
        // Ensure min and max are reset correctly
        min = Double.MAX_VALUE;
        max = -Double.MAX_VALUE;
    }

[codity-dev]

The function telemetry_log_set_async has a TODO comment indicating that the async queue should be initialized or destroyed based on the flag, but this logic is not implemented. This could lead to unexpected behavior if the async logging state is changed without properly handling the queue.

Code Suggestion or Comments

int telemetry_log_set_async(int async)
{
	global_context.async_logging = async;

	if (async)
	{
		// Initialize async queue
		// Example: global_context.async_queue = initialize_async_queue();
	}
	else
	{
		// Destroy async queue
		// Example: destroy_async_queue(global_context.async_queue);
	}

	return 0;
}

[codity-dev]

The function telemetry_log_flush has a TODO comment indicating that the async queue should be flushed if present, but this logic is not implemented. This could lead to data not being flushed properly if async logging is enabled.

Code Suggestion or Comments

int telemetry_log_flush(void)
{
	size_t i;

	if (global_context.async_logging)
	{
		// Flush async queue
		// Example: flush_async_queue(global_context.async_queue);
	}

	/* Flush all file handlers */
	for (i = 0; i < global_context.handler_count; ++i)
	{
		if (global_context.handlers[i] != NULL)
		{
			telemetry_log_handler handler = global_context.handlers[i];
			if (handler->file_handle != NULL)
			{
				fflush(handler->file_handle);
			}
		}
	}

	return 0;
}

[codity-dev]

The function telemetry_log_entry_format_text does not check the return value of snprintf for errors. If snprintf fails, it returns a negative value, which should be handled to avoid incorrect buffer manipulation.

Code Suggestion or Comments

static int telemetry_log_entry_format_text(telemetry_log_entry entry, char *buffer, size_t size, telemetry_log_formatter formatter)
{
	int written = 0;
	int n;

	if (entry == NULL || buffer == NULL || size == 0)
	{
		return -1;
	}

	buffer[0] = '\0';

	/* Timestamp */
	if (formatter->include_timestamp)
	{
		n = snprintf(buffer + written, size - written, "[%s] ", entry->timestamp);
		if (n < 0)
			return -1;
		written += n;
	}

	/* Log level */
	if (formatter->include_level)
	{
		n = snprintf(buffer + written, size - written, "[%s] ", telemetry_log_level_to_string(entry->level));
		if (n < 0)
			return -1;
		written += n;
	}

	/* Category */
	if (formatter->include_category && entry->category[0] != '\0')
	{
		n = snprintf(buffer + written, size - written, "[%s] ", entry->category);
		if (n < 0)
			return -1;
		written += n;
	}

	/* Thread info */
	if (formatter->include_thread_info)
	{
		n = snprintf(buffer + written, size - written, "[pid:%u tid:%llu] ", entry->process_id, (unsigned long long)entry->thread_id);
		if (n < 0)
			return -1;
		written += n;
	}

	/* Message */
	n = snprintf(buffer + written, size - written, "%s", entry->message);
	if (n < 0)
		return -1;
	written += n;

	/* Location */
	if (formatter->include_location && entry->file != NULL)
	{
		n = snprintf(buffer + written, size - written, " (%s:%d in %s)", entry->file, entry->line, entry->function ? entry->function : "?");
		if (n < 0)
			return -1;
		written += n;
	}

	return written;
}

[codity-dev]

The function telemetry_log_entry_format_json does not check the return value of snprintf for errors. If snprintf fails, it returns a negative value, which should be handled to avoid incorrect buffer manipulation.

Code Suggestion or Comments

static int telemetry_log_entry_format_json(telemetry_log_entry entry, char *buffer, size_t size, telemetry_log_formatter formatter)
{
	int written = 0;
	int n;

	if (entry == NULL || buffer == NULL || size == 0)
	{
		return -1;
	}

	n = snprintf(buffer + written, size - written, "{");
	if (n < 0)
		return -1;
	written += n;

	if (formatter->include_timestamp)
	{
		n = snprintf(buffer + written, size - written, "\"timestamp\":\"%s\",", entry->timestamp);
		if (n < 0)
			return -1;
		written += n;
	}

	if (formatter->include_level)
	{
		n = snprintf(buffer + written, size - written, "\"level\":\"%s\",", telemetry_log_level_to_string(entry->level));
		if (n < 0)
			return -1;
		written += n;
	}

	if (formatter->include_category)
	{
		n = snprintf(buffer + written, size - written, "\"category\":\"%s\",", entry->category);
		if (n < 0)
			return -1;
		written += n;
	}

	if (formatter->include_thread_info)
	{
		n = snprintf(buffer + written, size - written, "\"process_id\":%u,\"thread_id\":%llu,", entry->process_id, (unsigned long long)entry->thread_id);
		if (n < 0)
			return -1;
		written += n;
	}

	n = snprintf(buffer + written, size - written, "\"message\":\"%s\"");
	if (n < 0)
		return -1;
	written += n;

	if (formatter->include_location && entry->file != NULL)
	{
		n = snprintf(buffer + written, size - written, ",\"file\":\"%s\",\"line\":%d,\"function\":\"%s\"");
		if (n < 0)
			return -1;
		written += n;
	}

	n = snprintf(buffer + written, size - written, "}");
	if (n < 0)
		return -1;
	written += n;

	return written;
}

[codity-dev]

The function telemetry_log_entry_format_xml does not check the return value of snprintf for errors. If snprintf fails, it returns a negative value, which should be handled to avoid incorrect buffer manipulation.

Code Suggestion or Comments

static int telemetry_log_entry_format_xml(telemetry_log_entry entry, char *buffer, size_t size, telemetry_log_formatter formatter)
{
	int written = 0;
	int n;

	if (entry == NULL || buffer == NULL || size == 0)
	{
		return -1;
	}

	n = snprintf(buffer + written, size - written, "<log>");
	if (n < 0)
		return -1;
	written += n;

	if (formatter->include_timestamp)
	{
		n = snprintf(buffer + written, size - written, "<timestamp>%s</timestamp>", entry->timestamp);
		if (n < 0)
			return -1;
		written += n;
	}

	if (formatter->include_level)
	{
		n = snprintf(buffer + written, size - written, "<level>%s</level>", telemetry_log_level_to_string(entry->level));
		if (n < 0)
			return -1;
		written += n;
	}

	if (formatter->include_category)
	{
		n = snprintf(buffer + written, size - written, "<category>%s</category>", entry->category);
		if (n < 0)
			return -1;
		written += n;
	}

	if (formatter->include_thread_info)
	{
		n = snprintf(buffer + written, size - written, "<process_id>%u</process_id><thread_id>%llu</thread_id>", entry->process_id, (unsigned long long)entry->thread_id);
		if (n < 0)
			return -1;
		written += n;
	}

	n = snprintf(buffer + written, size - written, "<message>%s</message>", entry->message);
	if (n < 0)
		return -1;
	written += n;

	if (formatter->include_location && entry->file != NULL)
	{
		n = snprintf(buffer + written, size - written, "<location><file>%s</file><line>%d</line><function>%s</function></location>", entry->file, entry->line, entry->function ? entry->function : "?");
		if (n < 0)
			return -1;
		written += n;
	}

	n = snprintf(buffer + written, size - written, "</log>");
	if (n < 0)
		return -1;
	written += n;

	return written;
}

[codity-dev]

The function telemetry_log_entry_format_colored does not check the return value of snprintf for errors. If snprintf fails, it returns a negative value, which should be handled to avoid incorrect buffer manipulation.

Code Suggestion or Comments

static int telemetry_log_entry_format_colored(telemetry_log_entry entry, char *buffer, size_t size, telemetry_log_formatter formatter)
{
	int written = 0;
	int n;
	const char *color_code = telemetry_log_get_color_code(entry->level);
	const char *reset_code = telemetry_log_get_reset_code();

	if (entry == NULL || buffer == NULL || size == 0)
	{
		return -1;
	}

	buffer[0] = '\0';

	/* Start with color */
	n = snprintf(buffer + written, size - written, "%s", color_code);
	if (n < 0)
		return -1;
	written += n;

	/* Timestamp */
	if (formatter->include_timestamp)
	{
		n = snprintf(buffer + written, size - written, "[%s] ", entry->timestamp);
		if (n < 0)
			return -1;
		written += n;
	}

	/* Continue with other fields as needed */
}

[codity-dev]

The function telemetry_metric_export_json does not handle potential buffer overflow when using snprintf. If the buffer size is insufficient, snprintf will truncate the output, potentially leading to incomplete JSON data.

Code Suggestion or Comments

static int telemetry_metric_export_json(telemetry_metric metric, char *buffer, size_t size)
{
    int written = 0;
    int n;

    if (metric == NULL || buffer == NULL || size == 0)
    {
        return -1;
    }

    n = snprintf(buffer + written, size - written, "{\"name\":\"%s\",\"type\":\"%s\"",
        metric->name,
        metric->type == TELEMETRY_METRIC_TYPE_COUNTER ? "counter" : (metric->type == TELEMETRY_METRIC_TYPE_GAUGE ? "gauge" : "histogram"));
    if (n < 0 || n >= size - written)
        return -1;
    written += n;

    if (metric->description[0] != '\0')
    {
        n = snprintf(buffer + written, size - written, ",\"description\":\"%s\"", metric->description);
        if (n < 0 || n >= size - written)
            return -1;
        written += n;
    }

    if (metric->unit[0] != '\0')
    {
        n = snprintf(buffer + written, size - written, ",\"unit\":\"%s\"", metric->unit);
        if (n < 0 || n >= size - written)
            return -1;
        written += n;
    }

    /* Add value */
    switch (metric->type)
    {
        case TELEMETRY_METRIC_TYPE_COUNTER:
            n = snprintf(buffer + written, size - written, ",\"value\":%llu", (unsigned long long)metric->data.counter.value);
            if (n < 0 || n >= size - written)
                return -1;
            written += n;
            break;

        case TELEMETRY_METRIC_TYPE_GAUGE:
            n = snprintf(buffer + written, size - written, ",\"value\":%.6f,\"min\":%.6f,\"max\":%.6f",
                metric->data.gauge.value,
                metric->data.gauge.min_value,
                metric->data.gauge.max_value);
            if (n < 0 || n >= size - written)
                return -1;
            written += n;
            break;

        case TELEMETRY_METRIC_TYPE_HISTOGRAM:
            n = snprintf(buffer + written, size - written, ",\"count\":%llu,\"sum\":%.6f,\"min\":%.6f,\"max\":%.6f",
                (unsigned long long)metric->data.histogram.total_count,
                metric->data.histogram.sum,
                metric->data.histogram.min,
                metric->data.histogram.max);
            if (n < 0 || n >= size - written)
                return -1;
            written += n;
            break;

        default:
            break;
    }

    /* Add labels if present */
    if (metric->label_count > 0)
    {
        size_t i;
        n = snprintf(buffer + written, size - written, ",\"labels\":{");
        if (n < 0 || n >= size - written)
            return -1;
        written += n;

        for (i = 0; i < metric->label_count; ++i)
        {
            n = snprintf(buffer + written, size - written, "%s\"%s\":\"%s\"",
                i > 0 ? "," : "",
                metric->labels[i].key,
                metric->labels[i].value);
            if (n < 0 || n >= size - written)
                return -1;
            written += n;
        }

        n = snprintf(buffer + written, size - written, "}");
        if (n < 0 || n >= size - written)
            return -1;
        written += n;
    }

    n = snprintf(buffer + written, size - written, "}\n");
    if (n < 0 || n >= size - written)
        return -1;
    written += n;

    return written;
}

[codity-dev]

The function telemetry_metric_export_statsd does not handle potential buffer overflow when using snprintf. If the buffer size is insufficient, snprintf will truncate the output, potentially leading to incomplete data.

Code Suggestion or Comments

static int telemetry_metric_export_statsd(telemetry_metric metric, char *buffer, size_t size)
{
    int written = 0;

    if (metric == NULL || buffer == NULL || size == 0)
    {
        return -1;
    }

    switch (metric->type)
    {
        case TELEMETRY_METRIC_TYPE_COUNTER:
            written = snprintf(buffer, size, "%s:%llu|c\n", metric->name, (unsigned long long)metric->data.counter.value);
            if (written < 0 || written >= size)
                return -1;
            break;

        case TELEMETRY_METRIC_TYPE_GAUGE:
            written = snprintf(buffer, size, "%s:%.6f|g\n", metric->name, metric->data.gauge.value);
            if (written < 0 || written >= size)
                return -1;
            break;

        case TELEMETRY_METRIC_TYPE_HISTOGRAM:
            written = snprintf(buffer, size, "%s:%.6f|h\n", metric->name, metric->data.histogram.sum);
            if (written < 0 || written >= size)
                return -1;
            break;

        default:
            written = -1;
            break;
    }

    return written;
}

[codity-dev]

The function telemetry_metric_export_influxdb does not handle potential buffer overflow when using snprintf. If the buffer size is insufficient, snprintf will truncate the output, potentially leading to incomplete data.

Code Suggestion or Comments

static int telemetry_metric_export_influxdb(telemetry_metric metric, char *buffer, size_t size)
{
    int written = 0;
    int n;
    char labels_buf[512];

    if (metric == NULL || buffer == NULL || size == 0)
    {
        return -1;
    }

    /* Format labels as tags */
    telemetry_metric_labels_to_string(metric, labels_buf, sizeof(labels_buf), "influxdb");

    /* Measurement name */
    n = snprintf(buffer + written, size - written, "%s%s ", metric->name, labels_buf);
    if (n < 0 || n >= size - written)
        return -1;
    written += n;

    /* Fields */
    switch (metric->type)
    {
        case TELEMETRY_METRIC_TYPE_COUNTER:
            n = snprintf(buffer + written, size - written, "value=%llui", (unsigned long long)metric->data.counter.value);
            if (n < 0 || n >= size - written)
                return -1;
            written += n;
            break;

        case TELEMETRY_METRIC_TYPE_GAUGE:
            n = snprintf(buffer + written, size - written, "value=%.6f,min=%.6f,max=%.6f",
                metric->data.gauge.value,
                metric->data.gauge.min_value,
                metric->data.gauge.max_value);
            if (n < 0 || n >= size - written)
                return -1;
            written += n;
            break;

        case TELEMETRY_METRIC_TYPE_HISTOGRAM:
            n = snprintf(buffer + written, size - written, "count=%llui,sum=%.6f,min=%.6f,max=%.6f",
                (unsigned long long)metric->data.histogram.total_count,
                metric->data.histogram.sum,
                metric->data.histogram.min,
                metric->data.histogram.max);
            if (n < 0 || n >= size - written)
                return -1;
            written += n;
            break;

        default:
            break;
    }

    n = snprintf(buffer + written, size - written, "\n");
    if (n < 0 || n >= size - written)
        return -1;
    written += n;

    return written;
}

[codity-dev]

The function telemetry_metric_export_graphite does not handle potential buffer overflow when using snprintf. If the buffer size is insufficient, snprintf will truncate the output, potentially leading to incomplete data.

Code Suggestion or Comments

static int telemetry_metric_export_graphite(telemetry_metric metric, char *buffer, size_t size)
{
    int written = 0;
    time_t now = time(NULL);

    if (metric == NULL || buffer == NULL || size == 0)
    {
        return -1;
    }

    switch (metric->type)
    {
        case TELEMETRY_METRIC_TYPE_COUNTER:
            written = snprintf(buffer, size, "%s %llu %ld\n",
                metric->name,
                (unsigned long long)metric->data.counter.value,
                (long)now);
            if (written < 0 || written >= size)
                return -1;
            break;

        case TELEMETRY_METRIC_TYPE_GAUGE:
            written = snprintf(buffer, size, "%s %.6f %ld\n",
                metric->name,
                metric->data.gauge.value,
                (long)now);
            if (written < 0 || written >= size)
                return -1;
            break;

        case TELEMETRY_METRIC_TYPE_HISTOGRAM:
        {
            int n;
            n = snprintf(buffer + written, size - written, "%s.count %llu %ld\n",
                metric->name,
                (unsigned long long)metric->data.histogram.total_count,
                (long)now);
            if (n < 0 || n >= size - written)
                return -1;
            written += n;

            n = snprintf(buffer + written, size - written, "%s.sum %.6f %ld\n",
                metric->name,
                metric->data.histogram.sum,
                (long)now);
            if (n < 0 || n >= size - written)
                return -1;
            written += n;
            break;
        }

        default:
            written = -1;
            break;
    }

    return written;
}

[codity-dev]

The function telemetry_metric_escape_label_value does not handle potential null input for input parameter, which could lead to undefined behavior when calling strlen.

Code Suggestion or Comments

static void telemetry_metric_escape_label_value(const char *input, char *output, size_t output_size)
{
    size_t i, j;

    if (input == NULL || output == NULL || output_size == 0)
    {
        return;
    }

    size_t input_len = strlen(input);

    for (i = 0, j = 0; i < input_len && j < output_size - 1; ++i)
    {
        char c = input[i];

[codity-dev]

The test functions do not handle potential errors from telemetry functions, such as failures in metric creation or registration. Adding error handling would make the tests more robust.

Code Suggestion or Comments

telemetry_metric counter = telemetry_metric_counter_create("api_calls", "API calls counter", "calls");
if (counter == NULL) {
    fprintf(stderr, "Failed to create counter metric\n");
    return;
}

[codity-dev]

The test suite relies on a setTimeout to determine when tests are complete, which can lead to flaky tests if the timeout is not sufficient.

Code Suggestion or Comments

function runTests() {
	console.log(`\n=== Test Results ===`);
	console.log(`Tests passed: ${testsPassed}`);
	console.log(`Tests failed: ${testsFailed}`);

	if (testsFailed === 0) {
		console.log("\n=== All JavaScript Tests Passed ===");
		process.exit(0);
	} else {
		console.log("\n=== Some Tests Failed ===");
		process.exit(1);
	}
}

// Call runTests after all tests have been executed
runTests();

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_log_init_shutdown(void)
{
	printf("Testing log initialization and shutdown...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ Log initialization and shutdown test passed\n");
}

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_log_levels(void)
{
	printf("Testing log levels...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}

	telemetry_log_set_level(TELEMETRY_LOG_LEVEL_DEBUG);
	assert(telemetry_log_get_level() == TELEMETRY_LOG_LEVEL_DEBUG);

	telemetry_log_set_level(TELEMETRY_LOG_LEVEL_ERROR);
	assert(telemetry_log_get_level() == TELEMETRY_LOG_LEVEL_ERROR);

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ Log levels test passed\n");
}

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_log_level_strings(void)
{
	printf("Testing log level string conversion...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}

	assert(strcmp(telemetry_log_level_to_string(TELEMETRY_LOG_LEVEL_TRACE), "TRACE") == 0);
	assert(strcmp(telemetry_log_level_to_string(TELEMETRY_LOG_LEVEL_DEBUG), "DEBUG") == 0);
	assert(strcmp(telemetry_log_level_to_string(TELEMETRY_LOG_LEVEL_INFO), "INFO") == 0);
	assert(strcmp(telemetry_log_level_to_string(TELEMETRY_LOG_LEVEL_WARNING), "WARNING") == 0);
	assert(strcmp(telemetry_log_level_to_string(TELEMETRY_LOG_LEVEL_ERROR), "ERROR") == 0);
	assert(strcmp(telemetry_log_level_to_string(TELEMETRY_LOG_LEVEL_CRITICAL), "CRITICAL") == 0);

	assert(telemetry_log_level_from_string("TRACE") == TELEMETRY_LOG_LEVEL_TRACE);
	assert(telemetry_log_level_from_string("DEBUG") == TELEMETRY_LOG_LEVEL_DEBUG);
	assert(telemetry_log_level_from_string("INFO") == TELEMETRY_LOG_LEVEL_INFO);
	assert(telemetry_log_level_from_string("WARNING") == TELEMETRY_LOG_LEVEL_WARNING);
	assert(telemetry_log_level_from_string("ERROR") == TELEMETRY_LOG_LEVEL_ERROR);
	assert(telemetry_log_level_from_string("CRITICAL") == TELEMETRY_LOG_LEVEL_CRITICAL);

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ Log level string conversion test passed\n");
}

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_log_handlers(void)
{
	printf("Testing log handlers...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}

	telemetry_log_handler console_handler = telemetry_log_handler_create(
		TELEMETRY_LOG_HANDLER_CONSOLE,
		TELEMETRY_LOG_LEVEL_INFO);
	assert(console_handler != NULL);

	result = telemetry_log_handler_register(console_handler);
	assert(result == 0);

	result = telemetry_log_handler_unregister(console_handler);
	assert(result == 0);

	telemetry_log_handler_destroy(console_handler);

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ Log handlers test passed\n");
}

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_file_handler(void)
{
	printf("Testing file handler...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}

	telemetry_log_handler file_handler = telemetry_log_handler_create(
		TELEMETRY_LOG_HANDLER_FILE,
		TELEMETRY_LOG_LEVEL_DEBUG);
	assert(file_handler != NULL);

	result = telemetry_log_handler_configure_file(
		file_handler,
		"/tmp/test_telemetry.log",
		TELEMETRY_LOG_ROTATION_SIZE,
		1024 * 1024); /* 1MB */
	assert(result == 0);

	result = telemetry_log_handler_register(file_handler);
	assert(result == 0);

	telemetry_log_handler_unregister(file_handler);
	telemetry_log_handler_destroy(file_handler);

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ File handler test passed\n");
}

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_log_formatters(void)
{
	printf("Testing log formatters...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}

	telemetry_log_formatter text_formatter = telemetry_log_formatter_create(TELEMETRY_LOG_FORMAT_TEXT);
	assert(text_formatter != NULL);

	result = telemetry_log_formatter_configure(text_formatter, 1, 1, 1, 1, 0);
	assert(result == 0);

	telemetry_log_formatter_destroy(text_formatter);

	telemetry_log_formatter json_formatter = telemetry_log_formatter_create(TELEMETRY_LOG_FORMAT_JSON);
	assert(json_formatter != NULL);
	telemetry_log_formatter_destroy(json_formatter);

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ Log formatters test passed\n");
}

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_log_filters(void)
{
	printf("Testing log filters...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}

	telemetry_log_filter filter = telemetry_log_filter_create();
	assert(filter != NULL);

	result = telemetry_log_filter_set_category(filter, "test.*");
	assert(result == 0);

	result = telemetry_log_filter_set_level(filter, TELEMETRY_LOG_LEVEL_WARNING);
	assert(result == 0);

	result = telemetry_log_filter_register(filter);
	assert(result == 0);

	result = telemetry_log_filter_unregister(filter);
	assert(result == 0);

	telemetry_log_filter_destroy(filter);

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ Log filters test passed\n");
}

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_log_messages(void)
{
	printf("Testing log messages...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}
	telemetry_log_set_level(TELEMETRY_LOG_LEVEL_TRACE);

	telemetry_log_handler console_handler = telemetry_log_handler_create(
		TELEMETRY_LOG_HANDLER_CONSOLE,
		TELEMETRY_LOG_LEVEL_TRACE);

	telemetry_log_formatter formatter = telemetry_log_formatter_create(TELEMETRY_LOG_FORMAT_TEXT);
	telemetry_log_handler_set_formatter(console_handler, formatter);
	telemetry_log_handler_register(console_handler);

	TELEMETRY_LOG_TRACE("test", "This is a trace message");
	TELEMETRY_LOG_DEBUG("test", "This is a debug message");
	TELEMETRY_LOG_INFO("test", "This is an info message");
	TELEMETRY_LOG_WARNING("test", "This is a warning message");
	TELEMETRY_LOG_ERROR("test", "This is an error message");
	TELEMETRY_LOG_CRITICAL("test", "This is a critical message");

	telemetry_log_flush();

	telemetry_log_handler_unregister(console_handler);
	telemetry_log_handler_destroy(console_handler);

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ Log messages test passed\n");
}

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_colored_formatter(void)
{
	printf("Testing colored formatter...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}

	telemetry_log_handler console_handler = telemetry_log_handler_create(
		TELEMETRY_LOG_HANDLER_CONSOLE,
		TELEMETRY_LOG_LEVEL_TRACE);

	telemetry_log_formatter colored_formatter = telemetry_log_formatter_create(TELEMETRY_LOG_FORMAT_COLORED);
	telemetry_log_handler_set_formatter(console_handler, colored_formatter);
	telemetry_log_handler_register(console_handler);

	TELEMETRY_LOG_INFO("test", "Testing colored output");
	TELEMETRY_LOG_ERROR("test", "Testing colored error output");

	telemetry_log_handler_unregister(console_handler);
	telemetry_log_handler_destroy(console_handler);

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ Colored formatter test passed\n");
}

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_json_formatter(void)
{
	printf("Testing JSON formatter...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}

	telemetry_log_handler console_handler = telemetry_log_handler_create(
		TELEMETRY_LOG_HANDLER_CONSOLE,
		TELEMETRY_LOG_LEVEL_INFO);

	telemetry_log_formatter json_formatter = telemetry_log_formatter_create(TELEMETRY_LOG_FORMAT_JSON);
	telemetry_log_handler_set_formatter(console_handler, json_formatter);
	telemetry_log_handler_register(console_handler);

	TELEMETRY_LOG_INFO("test", "Testing JSON output");

	telemetry_log_handler_unregister(console_handler);
	telemetry_log_handler_destroy(console_handler);

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ JSON formatter test passed\n");
}

[codity-dev]

The test functions do not handle potential errors from telemetry_log_initialize() and telemetry_log_shutdown(). If these functions fail, subsequent operations may not behave as expected.

Code Suggestion or Comments

void test_xml_formatter(void)
{
	printf("Testing XML formatter...\n");

	int result = telemetry_log_initialize();
	if (result != 0) {
		fprintf(stderr, "Failed to initialize telemetry log\n");
		return;
	}

	telemetry_log_handler console_handler = telemetry_log_handler_create(
		TELEMETRY_LOG_HANDLER_CONSOLE,
		TELEMETRY_LOG_LEVEL_INFO);

	telemetry_log_formatter xml_formatter = telemetry_log_formatter_create(TELEMETRY_LOG_FORMAT_XML);
	telemetry_log_handler_set_formatter(console_handler, xml_formatter);
	telemetry_log_handler_register(console_handler);

	TELEMETRY_LOG_INFO("test", "Testing XML output");

	telemetry_log_handler_unregister(console_handler);
	telemetry_log_handler_destroy(console_handler);

	result = telemetry_log_shutdown();
	if (result != 0) {
		fprintf(stderr, "Failed to shutdown telemetry log\n");
		return;
	}

	printf("✓ XML formatter test passed\n");
}

[codity-dev]

The class contains TODO comments indicating that native C library functions should be called, but these are not yet implemented. This can lead to incomplete functionality and confusion for future developers.

Code Suggestion or Comments

// Implement the call to the native C library function here

[codity-dev]

The class contains TODO comments indicating that native C library functions should be called, but these are not yet implemented. This can lead to incomplete functionality and confusion for future developers.

Code Suggestion or Comments

// Implement the call to the native C library function here

[codity-dev]

The class contains TODO comments indicating that native C library functions should be called, but these are not yet implemented. This can lead to incomplete functionality and confusion for future developers.

Code Suggestion or Comments

// Implement the call to the native C library function here

[codity-dev]

The TODO comments indicate that native C library functions should be called, but no implementation is provided. This can lead to confusion about the intended functionality.

Code Suggestion or Comments

// TODO: Implement the call to the native C library function here

[codity-dev]

The TODO comments indicate that native C library functions should be called, but no implementation is provided. This can lead to confusion about the intended functionality.

Code Suggestion or Comments

// TODO: Implement the call to the native C library function here

[codity-dev]

The class contains several TODO comments indicating that native C library functions need to be called, but these are not yet implemented. This can lead to incomplete functionality and confusion for future developers.

Code Suggestion or Comments

public void configureFile(String filePath, RotationPolicy rotation, long maxSize) {
        if (handlerType != HandlerType.FILE) {
            throw new IllegalStateException("configureFile only works with FILE handlers");
        }
        this.filePath = filePath;
        this.rotation = rotation;
        this.maxFileSize = maxSize;
        // Implement the call to the native C library function here
    }

    public void setFormatter(LogFormatter formatter) {
        this.formatter = formatter;
        // Implement the call to the native C library function here
    }

    protected void register() {
        // Implement the call to the native C library function here
    }

    protected void unregister() {
        // Implement the call to the native C library function here
    }

[codity-dev]

The constructor for the Metric class assigns default empty strings to description and unit if they are null. This is a good practice for avoiding null pointer exceptions, but it might be more maintainable to document this behavior explicitly in the method comments.

Code Suggestion or Comments

/**
     * Constructs a Metric with the specified name, description, and unit.
     * If description or unit is null, they are initialized to empty strings.
     */
    protected Metric(String name, String description, String unit) {
        this.name = name;
        this.description = description != null ? description : "";
        this.unit = unit != null ? unit : "";
        this.labels = new HashMap<>();
    }

[codity-dev]

The addLabel method includes a TODO comment indicating that a native C library function should be called. This suggests incomplete functionality that should be addressed for full implementation.

Code Suggestion or Comments

public void addLabel(String key, String value) {
        labels.put(key, value);
        // TODO: Implement call to native C library function for label addition
    }

[codity-dev]

The register method includes a TODO comment indicating that a native C library function should be called. This suggests incomplete functionality that should be addressed for full implementation.

Code Suggestion or Comments

protected void register() {
        // TODO: Implement call to native C library function for registration
    }

[codity-dev]

The unregister method includes a TODO comment indicating that a native C library function should be called. This suggests incomplete functionality that should be addressed for full implementation.

Code Suggestion or Comments

protected void unregister() {
        // TODO: Implement call to native C library function for unregistration
    }

[codity-dev]

🔍 PR review is completed!

[Dhiren-codity]

@codity review

[codity-dev]

🔍 PR review started! Estimated time: 20-25 minutes.

💡 Learn More

Ask Codity questions:
Mention @codity {your question} in a comment to get answers about the code.

Trigger a manual review:
Comment @codity review on a PR or MR.

View Full Docs

[codity-dev]

🔍 PR review is completed!