Enhance vts_zone directive with proper implementation and documentation #1

u5surf · 2025-07-25T23:00:39Z

Summary

Implement comprehensive vts_zone directive with zone name and size parsing
Add support for size units (k/K, m/M, g/G) with validation and error handling
Update README.md with detailed directive documentation and usage examples
Add shared memory zone initialization and proper configuration validation

Changes Made

Core Implementation (`src/lib.rs`)

Enhanced ngx_http_set_vts_zone function to properly parse directive arguments
Added parse_size_string helper function supporting k/m/g size units
Implemented vts_init_shm_zone callback for shared memory initialization
Added comprehensive error handling for invalid configurations
Included unit tests for size parsing functionality

Documentation (`README.md`)

Added detailed vts_zone directive documentation with parameters and examples
Updated configuration examples showing proper vts_zone main 10m usage
Enhanced architecture section explaining shared memory configuration
Improved directive reference with complete parameter descriptions

Key Features

Parameter Validation: Validates zone name and size arguments
Size Unit Support: Supports k/K (KB), m/M (MB), g/G (GB) units
Error Handling: Comprehensive error messages for invalid configurations
Shared Memory: Proper nginx shared memory zone creation and initialization
Testing: Unit tests ensure reliable size parsing functionality

Test Plan

All existing tests pass
New unit tests for parse_size_string function pass
Project builds successfully without errors
README documentation is comprehensive and accurate
Error handling tested for invalid configurations

Configuration Example

http {
    # Configure shared memory zone for VTS statistics
    vts_zone main 10m;
    
    server {
        location /status {
            vts_status;
            allow 127.0.0.1;
            deny all;
        }
    }
}

This enhancement provides a solid foundation for nginx-module-vts functionality with proper documentation and robust configuration handling.

🤖 Generated with Claude Code

Add foundational VTS (Virtual Host Traffic Status) node implementation similar to the original nginx-module-vts: ## New Features: - **VtsNodeStats**: Comprehensive traffic statistics structure - Request counts and timing metrics - Status code distribution (1xx-5xx) - Bytes in/out tracking - First/last request timestamps - **VtsStatsManager**: Statistics management system - Per-server zone tracking - In-memory storage (prepared for shared memory upgrade) - Thread-safe operations ready - **VTS Configuration**: Module directives - `vts_status` - Enable status endpoint - `vts_zone` - Configure shared memory zone (placeholder) ## Architecture Design: - Modular structure ready for shared memory integration - Red-black tree and slab allocator patterns researched - Compatible with original nginx-module-vts node system - Prepared for ngx-rust shared memory APIs ## Implementation Status: - ✅ Core statistics data structure - ✅ Basic statistics collection - ✅ Module configuration framework - 🔄 Shared memory integration (next phase) - 🔄 Request tracking hooks (next phase) This provides the foundation for efficient, persistent virtual host traffic monitoring with nginx's shared memory system. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Implement proper parsing of zone name and size parameters for vts_zone directive - Add support for size units (k/K, m/M, g/G) with proper validation - Add comprehensive error handling for invalid configurations - Update README.md with detailed vts_zone directive documentation - Add shared memory zone initialization callback - Include unit tests for size parsing functionality - Improve configuration examples and architecture documentation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Apply cargo fmt to resolve CI formatting check failures. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Add #[allow(dead_code)] annotations for unused structs and methods - Remove unnecessary type casts (usize -> usize) - Replace or_insert_with with or_default() for better performance - Use &raw const for mutable static reference to fix safety warning - Ensure all clippy warnings are resolved for CI compliance 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

Pull Request Overview

This pull request implements a comprehensive vts_zone directive for the nginx-module-vts Rust implementation, adding proper shared memory configuration capabilities. The changes establish the foundation for traffic statistics collection by enabling zone name and size parsing with support for standard memory units.

Enhanced the vts_zone directive implementation with proper argument parsing and validation
Added a new VTS node system for statistics data structures and management
Updated documentation with detailed directive usage and configuration examples

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
`src/vts_node.rs`	Introduces VTS statistics data structures and a temporary in-memory stats manager
`src/lib.rs`	Implements `vts_zone` directive parsing, shared memory zone creation, and size unit parsing
`README.md`	Updates documentation with `vts_zone` directive details and enhanced configuration examples

Comments suppressed due to low confidence (1)

src/lib.rs:184

[nitpick] The error message should be consistent with nginx's standard error message format. Consider using "takes" instead of "requires" to match nginx's typical phrasing: "vts_zone directive takes exactly 2 arguments: zone_name and size".

        let error_msg = "vts_zone directive requires exactly 2 arguments: zone_name and size\0";

src/lib.rs

Copilot · 2025-07-26T01:53:38Z

src/vts_node.rs

+        }
+
+        // Update timestamps
+        let current_time = ngx_time() as u64;


The ngx_time() function is called without proper safety checks. This is an unsafe FFI call that should be wrapped in an unsafe block, and the function should verify that nginx's time system is properly initialized.

Copilot · 2025-07-26T01:53:38Z

src/vts_node.rs

+
+/// Simple VTS statistics manager (without shared memory for now)
+///
+/// This will be replaced with shared memory implementation later


The comment indicates this is a temporary implementation that will be replaced. Consider adding a TODO or FIXME annotation to make this more explicit for future development: /// TODO: Replace with shared memory implementation.

Suggested change

/// This will be replaced with shared memory implementation later

/// TODO: Replace with shared memory implementation

Copilot · 2025-07-26T01:53:38Z

src/lib.rs

+
+    let num: usize = num_str.parse().map_err(|_| "Invalid number")?;
+
+    num.checked_mul(multiplier).ok_or("Size overflow")


The magic number multipliers (1024, 1024*1024, etc.) should be defined as named constants for better maintainability. Consider defining const KILOBYTE: usize = 1024;, const MEGABYTE: usize = 1024 * 1024;, etc.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

u5surf and others added 5 commits July 22, 2025 10:19

Fix code formatting issues

43ccc2a

Apply cargo fmt to resolve CI formatting check failures. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Fix cargo fmt

d8f683d

u5surf requested a review from Copilot July 26, 2025 01:52

Copilot AI reviewed Jul 26, 2025

View reviewed changes

u5surf and others added 2 commits July 26, 2025 11:00

Fix minimum version

c7173f2

Change Box::new(cstr)

cc976df

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

u5surf merged commit 98106f8 into main Jul 26, 2025
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enhance vts_zone directive with proper implementation and documentation #1

Enhance vts_zone directive with proper implementation and documentation #1

Uh oh!

u5surf commented Jul 25, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Jul 26, 2025

Uh oh!

Copilot AI Jul 26, 2025

Uh oh!

Copilot AI Jul 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	/// This will be replaced with shared memory implementation later
	/// TODO: Replace with shared memory implementation


		let num: usize = num_str.parse().map_err(\|_\| "Invalid number")?;

		num.checked_mul(multiplier).ok_or("Size overflow")

Enhance vts_zone directive with proper implementation and documentation #1

Enhance vts_zone directive with proper implementation and documentation #1

Uh oh!

Conversation

u5surf commented Jul 25, 2025

Summary

Changes Made

Core Implementation (src/lib.rs)

Documentation (README.md)

Key Features

Test Plan

Configuration Example

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Jul 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Core Implementation (`src/lib.rs`)

Documentation (`README.md`)