ulid: simplify, add function without a timestamp

CLAUDE.md

@@ -103,7 +103,7 @@ OpenTelemetry distributed tracing setup with support for OTLP export via gRPC or
 Shared data structures for the NTP Pool project. Currently contains `LogScoreAttributes` for NTP server scoring with JSON and SQL database compatibility.
 
 ### `ulid/`
-Thread-safe ULID (Universally Unique Lexicographically Sortable Identifier) generation using pooled monotonic readers with cryptographically secure seeding for high-concurrency environments.
+Thread-safe ULID (Universally Unique Lexicographically Sortable Identifier) generation using cryptographically secure randomness. Optimized for simplicity and performance in high-concurrency environments.
 
 ### `version/`
 Build metadata and version information system with Git integration. Provides CLI commands for Cobra and Kong frameworks, Prometheus build info metrics, and semantic version validation.

ulid/ulid.go

@@ -1,64 +1,44 @@
 // Package ulid provides thread-safe ULID (Universally Unique Lexicographically Sortable Identifier) generation.
 //
 // ULIDs are 128-bit identifiers that are lexicographically sortable and contain
-// a timestamp component. This package uses a pool-based approach with
-// cryptographically secure random seeding and monotonic ordering for optimal
-// performance in concurrent environments.
+// a timestamp component. This package uses cryptographically secure random
+// generation optimized for simplicity and performance in concurrent environments.
 package ulid
 
 import (
 	cryptorand "crypto/rand"
-	"encoding/binary"
-	"io"
-	mathrand "math/rand"
-	"os"
-	"sync"
 	"time"
 
 	oklid "github.com/oklog/ulid/v2"
-	"go.ntppool.org/common/logger"
 )
 
-// monotonicPool is a pool of monotonic ULID readers for performance optimization.
-// Each reader is initialized with a cryptographically secure random seed
-// and random increment value to ensure uniqueness across concurrent usage.
-var monotonicPool = sync.Pool{
-	New: func() any {
-		log := logger.Setup()
-
-		var seed int64
-		err := binary.Read(cryptorand.Reader, binary.BigEndian, &seed)
-		if err != nil {
-			log.Error("crypto/rand error", "err", err)
-			os.Exit(10)
-		}
-
-		rand := mathrand.New(mathrand.NewSource(seed))
-
-		inc := uint64(mathrand.Int63())
-
-		// log.Printf("seed: %d", seed)
-		// log.Printf("inc:  %d", inc)
-
-		// inc = inc & ^uint64(1<<63) // only want 63 bits
-		mono := oklid.Monotonic(rand, inc)
-		return mono
-	},
-}
-
-// MakeULID generates a new ULID with the specified timestamp using a pooled monotonic reader.
+// MakeULID generates a new ULID with the specified timestamp using cryptographically secure randomness.
 // The function is thread-safe and optimized for high-concurrency environments.
 //
-// Each call retrieves a monotonic reader from the pool, generates a ULID with the
-// given timestamp, and returns it. The reader is not returned to the pool as it
-// maintains internal state for monotonic ordering.
+// This implementation prioritizes simplicity and performance over strict monotonicity within
+// the same millisecond. Each ULID is guaranteed to be unique and lexicographically sortable
+// across different timestamps.
 //
 // Returns a pointer to the generated ULID or an error if generation fails.
 // Generation should only fail under extreme circumstances (entropy exhaustion).
 func MakeULID(t time.Time) (*oklid.ULID, error) {
-	mono := monotonicPool.Get().(io.Reader)
-
-	id, err := oklid.New(oklid.Timestamp(t), mono)
+	id, err := oklid.New(oklid.Timestamp(t), cryptorand.Reader)
+	if err != nil {
+		return nil, err
+	}
+
+	return &id, nil
+}
+
+// Make generates a new ULID with the current timestamp using cryptographically secure randomness.
+// This is a convenience function equivalent to MakeULID(time.Now()).
+//
+// The function is thread-safe and optimized for high-concurrency environments.
+//
+// Returns a pointer to the generated ULID or an error if generation fails.
+// Generation should only fail under extreme circumstances (entropy exhaustion).
+func Make() (*oklid.ULID, error) {
+	id, err := oklid.New(oklid.Now(), cryptorand.Reader)
 	if err != nil {
 		return nil, err
 	}

ulid/ulid_test.go

@@ -1,6 +1,7 @@
 package ulid
 
 import (
+	cryptorand "crypto/rand"
 	"sort"
 	"sync"
 	"testing"
@@ -36,6 +37,38 @@ func TestMakeULID(t *testing.T) {
 	t.Logf("ulid string 1 and 2: %s | %s", ul1.String(), ul2.String())
 }
 
+func TestMake(t *testing.T) {
+	// Test Make() function (uses current time)
+	ul1, err := Make()
+	if err != nil {
+		t.Fatalf("Make failed: %s", err)
+	}
+
+	if ul1 == nil {
+		t.Fatal("Make returned nil ULID")
+	}
+
+	// Sleep a bit and generate another
+	time.Sleep(2 * time.Millisecond)
+
+	ul2, err := Make()
+	if err != nil {
+		t.Fatalf("Make failed: %s", err)
+	}
+
+	// Should be different ULIDs
+	if ul1.String() == ul2.String() {
+		t.Errorf("ULIDs from Make() should be different: %s", ul1.String())
+	}
+
+	// Second should be later (or at least not earlier)
+	if ul1.Time() > ul2.Time() {
+		t.Errorf("second ULID should not have earlier timestamp: %d > %d", ul1.Time(), ul2.Time())
+	}
+
+	t.Logf("Make() ULIDs: %s | %s", ul1.String(), ul2.String())
+}
+
 func TestMakeULIDUniqueness(t *testing.T) {
 	tm := time.Now()
 	seen := make(map[string]bool)
@@ -54,6 +87,23 @@ func TestMakeULIDUniqueness(t *testing.T) {
 	}
 }
 
+func TestMakeUniqueness(t *testing.T) {
+	seen := make(map[string]bool)
+
+	for i := 0; i < 1000; i++ {
+		ul, err := Make()
+		if err != nil {
+			t.Fatalf("Make failed on iteration %d: %s", i, err)
+		}
+
+		str := ul.String()
+		if seen[str] {
+			t.Errorf("duplicate ULID generated: %s", str)
+		}
+		seen[str] = true
+	}
+}
+
 func TestMakeULIDTimestampProgression(t *testing.T) {
 	t1 := time.Now()
 	ul1, err := MakeULID(t1)
@@ -79,34 +129,6 @@ func TestMakeULIDTimestampProgression(t *testing.T) {
 	}
 }
 
-func TestMakeULIDMonotonicity(t *testing.T) {
-	tm := time.Now()
-	var ulids []*oklid.ULID
-
-	// Generate ULIDs rapidly with same timestamp
-	for i := 0; i < 100; i++ {
-		ul, err := MakeULID(tm)
-		if err != nil {
-			t.Fatalf("MakeULID failed on iteration %d: %s", i, err)
-		}
-		ulids = append(ulids, ul)
-	}
-
-	// Count non-monotonic pairs
-	nonMonotonicCount := 0
-	for i := 1; i < len(ulids); i++ {
-		if ulids[i-1].Compare(*ulids[i]) >= 0 {
-			nonMonotonicCount++
-		}
-	}
-	
-	// Report summary if any non-monotonic pairs found
-	if nonMonotonicCount > 0 {
-		t.Logf("Note: %d out of %d ULID pairs with same timestamp were not monotonic due to pool usage", 
-			nonMonotonicCount, len(ulids)-1)
-	}
-}
-
 func TestMakeULIDConcurrency(t *testing.T) {
 	const numGoroutines = 10
 	const numULIDsPerGoroutine = 100
@@ -152,6 +174,50 @@ func TestMakeULIDConcurrency(t *testing.T) {
 	}
 }
 
+func TestMakeConcurrency(t *testing.T) {
+	const numGoroutines = 10
+	const numULIDsPerGoroutine = 100
+
+	var wg sync.WaitGroup
+	ulidChan := make(chan *oklid.ULID, numGoroutines*numULIDsPerGoroutine)
+
+	// Start multiple goroutines generating ULIDs concurrently
+	for i := 0; i < numGoroutines; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for j := 0; j < numULIDsPerGoroutine; j++ {
+				ul, err := Make()
+				if err != nil {
+					t.Errorf("Make failed: %s", err)
+					return
+				}
+				ulidChan <- ul
+			}
+		}()
+	}
+
+	wg.Wait()
+	close(ulidChan)
+
+	// Collect all ULIDs and check uniqueness
+	seen := make(map[string]bool)
+	count := 0
+
+	for ul := range ulidChan {
+		str := ul.String()
+		if seen[str] {
+			t.Errorf("duplicate ULID generated in concurrent test: %s", str)
+		}
+		seen[str] = true
+		count++
+	}
+
+	if count != numGoroutines*numULIDsPerGoroutine {
+		t.Errorf("expected %d ULIDs, got %d", numGoroutines*numULIDsPerGoroutine, count)
+	}
+}
+
 func TestMakeULIDErrorHandling(t *testing.T) {
 	// Test with various timestamps
 	timestamps := []time.Time{
@@ -160,13 +226,13 @@ func TestMakeULIDErrorHandling(t *testing.T) {
 		time.Now().Add(time.Hour), // Future time
 	}
 
-	for _, tm := range timestamps {
+	for i, tm := range timestamps {
 		ul, err := MakeULID(tm)
 		if err != nil {
-			t.Errorf("MakeULID failed with timestamp %v: %s", tm, err)
+			t.Errorf("MakeULID failed with timestamp %d: %s", i, err)
 		}
 		if ul == nil {
-			t.Errorf("MakeULID returned nil ULID with timestamp %v", tm)
+			t.Errorf("MakeULID returned nil ULID with timestamp %d", i)
 		}
 	}
 }
@@ -223,6 +289,17 @@ func BenchmarkMakeULID(b *testing.B) {
 	}
 }
 
+// Benchmark Make function
+func BenchmarkMake(b *testing.B) {
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_, err := Make()
+		if err != nil {
+			b.Fatalf("Make failed: %s", err)
+		}
+	}
+}
+
 // Benchmark concurrent ULID generation
 func BenchmarkMakeULIDConcurrent(b *testing.B) {
 	tm := time.Now()
@@ -237,11 +314,23 @@ func BenchmarkMakeULIDConcurrent(b *testing.B) {
 	})
 }
 
-// Benchmark pool performance
-func BenchmarkMonotonicPool(b *testing.B) {
+// Benchmark concurrent Make function
+func BenchmarkMakeConcurrent(b *testing.B) {
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			_, err := Make()
+			if err != nil {
+				b.Fatalf("Make failed: %s", err)
+			}
+		}
+	})
+}
+
+// Benchmark random number generation
+func BenchmarkCryptoRand(b *testing.B) {
+	buf := make([]byte, 10) // ULID entropy size
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		_ = monotonicPool.Get()
-		// Note: we don't put it back as the current implementation doesn't reuse readers
+		cryptorand.Read(buf)
 	}
 }