Closed
Conversation
git-subtree-dir: lib/steno git-subtree-split: 5b08405f9e657fe3426d6ee2dedece383c201f73
Keep only the library code needed for CCNG integration. Removed: - .github/ (CI workflows, dependabot) - spec/ (will be tested via CCNG specs) - bin/ (steno-prettify tool) - Gem-related files (gemspec, Gemfile, Rakefile) - Documentation (README, CHANGELOG, RELEASING) - Config files (.rubocop*, .ruby-version, .gitignore) Kept: - lib/ (core steno library code) - LICENSE (Apache 2.0 attribution)
- Moved lib/steno/lib/steno/* to lib/steno/ for cleaner structure - Integrated codec_rfc3339.rb into steno's codec directory - Now lib/steno/ contains the steno library directly
Add STENO_HISTORY.md preserving the complete commit history (102 commits) from the original steno repository. This provides historical context and attribution for the integrated code, even though git subtree --squash collapsed the commits into a single merge commit.
Add back the 19 spec files (~1,115 lines) from the steno repository. These unit tests provide: - Direct testing of steno internal behavior - Regression protection when modifying steno code - Documentation of how the library works - Faster feedback than integration tests The specs can be run as part of CCNG's test suite to ensure the integrated steno library continues to work correctly.
Fixed 73 autocorrectable offenses: - Layout/SpaceAroundEqualsInParameterDefault - Layout/DotPosition - Style/FormatString - Style/RaiseArgs - Style/HashSyntax - Rails/Delegate - Lint/RedundantRequireStatement - Style/ArgumentsForwarding - Naming/BlockForwarding Remaining non-autocorrectable offenses: - Cop/PreferOjOverOtherJsonLibraries (Yajl usage) - Naming/MethodParameterName (short param names) - RSpec style issues - Style/FormatStringToken (format string annotations)
Changes: - Removed 'gem steno' from Gemfile (now using integrated version) - Updated lib/steno/codec.rb to require codec_rfc3339 - Simplified steno_configurer.rb requires (just require 'steno') - Removed redundant require statements from specs The steno library is now fully integrated into CCNG at lib/steno/ and no longer depends on the external gem.
Changes: - Added lib/steno.rb as the main entry point (requires lib/steno/steno.rb) - Added yajl-ruby ~> 1.0 to Gemfile (steno dependency) - Fixed missing ActiveSupport delegation requires in log_level.rb and core_ext.rb - Updated steno spec_helper to load only steno support files - Fixed require paths in all steno specs to use require_relative - Updated Gemfile.lock with yajl-ruby Result: All 79 steno unit tests now pass successfully!
Changes: - Moved tests from lib/steno/spec/ to spec/unit/lib/steno/ - Updated all test files to require main spec_helper first - Converted top-level describe blocks to RSpec.describe - Updated shared_examples to RSpec.shared_examples - Modified steno spec_helper to only load support files (not configure RSpec) - Tests now run as part of CCNG's normal test suite Result: All 86 tests pass (79 steno unit tests + 7 existing steno tests)
Add RSpec after hook to close syslog after each steno test. This prevents 'syslog already open' errors when running runner_spec or other tests that use syslog after the steno unit tests. The issue was that Syslog is a singleton that can only be opened once per process, and steno tests were leaving it open.
Windows support is unnecessary as CCNG only runs on Linux. This commit removes all Windows-specific code to simplify maintenance. Changes: - Removed lib/steno/sink/eventlog.rb (Windows Event Log sink) - Removed spec/unit/lib/steno/unit/sink/eventlog_spec.rb - Removed WINDOWS constant from sink/base.rb - Removed Windows conditionals from config.rb (eventlog config) - Removed Windows conditional wrappers from syslog.rb and tests - Removed require for eventlog from sink.rb - Fixed counter.rb to use string keys instead of symbols for Oj compatibility - Updated STENO_HISTORY.md to document all modifications made after integration: - JSON library migration (Yajl → Oj) - Syslog reopening fix - RuboCop compliance changes - Test structure updates - Windows support removal - Clarified LICENSE file is copied from original repository All 86 steno tests passing.
Apache 2.0 license requires distributing the NOTICE file along with LICENSE for proper attribution. This file contains copyright notices for the original steno project.
A README.md is more standard and discoverable than a HISTORY file. The README clearly documents: - This is a modified version of the original steno library - Integration rationale and details - All modifications made after integration - Complete original commit history - License and copyright information Since both CCNG and Steno are maintained by the Cloud Foundry Foundation, the integration and modifications are performed by the same organization.
Removed ~270 lines of unused code to simplify maintenance: Removed files: - lib/steno/version.rb - VERSION constant unused (no longer a gem) - lib/steno/json_prettifier.rb - CLI prettifier tool, ~130 lines - lib/steno/core_ext.rb - Monkey patches unused by CCNG - spec/unit/lib/steno/unit/json_prettifier_spec.rb - Tests for removed feature - spec/unit/lib/steno/unit/core_ext_spec.rb - Tests for removed feature - Removed require for version.rb from steno.rb Kept (even if unused): - TaggedLogger - Part of public Logger#tag() API - Logger#log_exception() - Useful utility method - Context::FiberLocal - Valid alternative, minimal code Updated README.md: - Documented all removed features in modifications section - Added "Making Future Modifications" section with guidelines - Added reminder to document changes in README when modifying steno Tests: 78 examples passing (was 86, removed 8 test examples for deleted features)
After deeper analysis, removed ~180 lines of additional unused code: Removed features: - Fluentd sink - never configured or used (~60 lines) - TaggedLogger class and Logger#tag() - never called (~64 lines) - Logger#log_exception() - never called (4 lines) - Context::FiberLocal - only ThreadLocal used (~15 lines + Fiber patches) - Steno.set_logger_regexp() - dynamic level adjustment, unused (~20 lines) - Steno.clear_logger_regexp() - unused (~15 lines) - Steno.logger_level_snapshot() - unused (~10 lines) Modified files: - lib/steno/config.rb - Removed Fluentd sink configuration - lib/steno/context.rb - Removed FiberLocal class and Fiber monkey patches - lib/steno/logger.rb - Removed tag() and log_exception() methods - lib/steno/sink.rb - Removed require for fluentd - lib/steno/steno.rb - Removed regexp methods, simplified logger() - Removed requires for tagged_logger from steno.rb - Updated tests to remove tests for deleted features Kept (used or needed): - Codec::Json - Backward compatibility fallback - Context::Null - Default fallback - All log levels - debug2 is actively used Total code reduction: - Round 1: ~240 lines - Round 2: ~180 lines - Total: ~420 lines removed Tests: 64 examples passing (was 78, removed 14 test examples) Updated README.md with complete documentation of Round 2 removals.
The test was checking for Float specifically, but Oj.load can parse JSON numbers as either Float or BigDecimal depending on the value and Oj configuration. Changed to check for Numeric which accepts both types. This fixes the CI test failure where BigDecimal was returned instead of Float.
- Add syslog gem to silence Ruby 3.4.0 deprecation warning Syslog is being removed from Ruby's default gems in 3.4.0 and needs to be explicitly added to the Gemfile. - Remove yajl-ruby gem as it's no longer used All Yajl usage was replaced with Oj in commit f122c77 but the gem was left in the Gemfile. The gem is completely unused in both application and test code (only referenced in linter tests that check for Yajl usage to prevent it).
The Syslog sink is a singleton that stores the @syslog instance variable. When tests mock Syslog.open, the mock gets stored in the singleton and persists across tests, causing RSpec mock errors when the next test tries to use the now-invalid mock. Fix by resetting the @syslog instance variable after each test in both: - spec_helper.rb: Global after hook for all tests - syslog_spec.rb: Specific after hook for syslog sink tests This fixes hundreds of CI test failures where tests were failing with: "#<Double "syslog"> was originally created in one example but has leaked into another example and can no longer be used."
Clarify that the ActiveSupport delegation require was added specifically for the Rails/Delegate cop auto-correction in log_level.rb, which converted 'def to_s; @name.to_s; end' to 'delegate :to_s, to: :@name'. This makes it clearer why the ActiveSupport dependency was introduced and which specific file uses it.
johha
force-pushed
the
integrate-steno-into-ccng
branch
from
de9ba3e to
436a6be
Compare
Prevent errors when @syslog is nil (e.g., when the sink hasn't been opened). This allows the sink to gracefully handle uninitialized state instead of raising: "undefined method `log' for nil" Also remove the singleton reset from syslog_spec after hook since the nil guard makes it unnecessary and it was interfering with other tests that mock the singleton with exact call expectations.
johha
force-pushed
the
integrate-steno-into-ccng
branch
from
436a6be to
060d846
Compare
The global after hook that resets the Syslog singleton was causing sporadic failures in parallel test runs, particularly in DelayedWorker specs where Steno context data was being lost. Changes: - Skip syslog reset when TEST_ENV_NUMBER is set (parallel execution) - Add :skip_syslog_reset tag to DelayedWorker spec for extra safety - Prevents race conditions when multiple test processes access singleton The nil guard in add_record ensures tests still work even if @syslog is not reset between tests in parallel mode.
johha
force-pushed
the
integrate-steno-into-ccng
branch
from
2c0efb6 to
d064d47
Compare
The previous approach of skipping syslog reset during parallel execution (TEST_ENV_NUMBER check) was incorrect - it actually prevented the cleanup that was needed, causing sporadic "double leaked into another example" errors when syslog_spec tests ran before other tests that trigger logging. Changes: - Add thread-safe reset! method to Steno::Sink::Syslog singleton - Simplify spec_helper after hook to always call reset! - Remove incorrect TEST_ENV_NUMBER check (parallel processes have separate singletons) - Keep :skip_syslog_reset tag for tests that mock the singleton class itself - Remove unnecessary :skip_syslog_reset from delayed_worker_spec The reset! method provides a clean API for clearing @syslog and @codec instance variables that may hold expired RSpec doubles.
johha
force-pushed
the
integrate-steno-into-ccng
branch
from
d064d47 to
c74756f
Compare
The Steno::Sink::Syslog singleton was causing sporadic test failures when steno tests ran in parallel with other CCNG tests. RSpec doubles stored in the singleton would leak into other tests, causing "double leaked into another example" errors. Instead of adding workarounds to spec_helper.rb, isolate the steno tests by moving them to spec/isolated_specs/steno/. This follows the existing pattern for tests that affect global state. Changes: - Move spec/unit/lib/steno/ to spec/isolated_specs/steno/ - Update steno spec_helper to call reset! and close Syslog after tests - Remove global syslog reset hook from main spec_helper.rb - Remove :skip_syslog_reset tags (no longer needed) The reset! method on Steno::Sink::Syslog remains as a clean API for test cleanup within the isolated steno test suite.
jochenehret
reviewed
Mar 2, 2026
jochenehret
previously approved these changes
Mar 3, 2026
johha
dismissed
jochenehret’s stale review
The merge-base changed after approval.
johha
force-pushed
the
integrate-steno-into-ccng
branch
2 times, most recently
from
975d9b0 to
cf8bc6b
Compare
Contributor
Author
|
Closed in favor #4902 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integrate the Steno logging library directly into CCNG from the standalone gem to reduce annual dependency maintenance overhead. Remove unused features and fix test issues.
Eliminates the need for annual dependency updates on a stable, rarely-changing library while maintaining the same logging functionality.
None
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the
mainbranchI have run all the unit tests using
bundle exec rakeI have run CF Acceptance Tests