Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.43%. Comparing base (89ca91d) to head (48d3c24).
⚠️ Report is 3 commits behind head on main.

@@             Coverage Diff              @@
##               main    #1768      +/-   ##
============================================
+ Coverage     68.38%   68.43%   +0.04%     
  Complexity     2970     2970              
============================================
  Files           448      448              
  Lines          8712     8714       +2     
============================================
+ Hits           5958     5963       +5     
+ Misses         2754     2751       -3     

This was done on purpose to ensure that we don't drop the event.name attribute.

Previously, event.name attribute could be replaced if $attributes contains another event.name in setAttributes call

$logRecord->setAttributes($attributes);

While the code set the event.name again after the setAttributes call to ensure we don't lose that attribute, so I think we don't need the first event.name set attributes calls. (The later always wins)

This was done on purpose to ensure that we don't drop the event.name attribute.

I have a memory of this, too. There is a unit test for it, and if that is passing then I think this change is OK now.

The linked test verifies that the $name parameter takes precedence over an event.name attribute provided in $attributes (= the second $logRecord->setAttribute('event.name', $name)).

The first $logRecord->setAttribute('event.name', $name) ensures that event.name is the first attribute entry -> is not dropped by attribute limits (unless 0).

#[BackupGlobals(true)]
public function test_event_name_is_not_dropped_first(): void
{
	$_SERVER['OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT'] = 1;
	$eventLoggerProvider = new EventLoggerProvider((new LoggerProviderBuilder())
		->addLogRecordProcessor(new SimpleLogRecordProcessor($exporter = new InMemoryExporter()))
		->build());
	$eventLogger = $eventLoggerProvider->getEventLogger('test');

	Logging::disable();
	try {
		$eventLogger->emit('my.event', attributes: ['other.attribute' => 'not.my.event']);
	} finally {
		Logging::reset();
	}
	$eventLoggerProvider->forceFlush();

	/** @var list<ReadableLogRecord> $logs */
	$logs = $exporter->getStorage()->getArrayCopy();
	$this->assertCount(1, $logs);
	$this->assertArrayHasKey('event.name', $logs[0]->getAttributes()->toArray());
}

Closing this PR as the function call is intended to set the first entry of the attributes so that it won't get dropped due to OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT scenario

Does this warrant adding a comment to the existing code to explain the scenario that is being covered by this?

@jerrytfleung thanks for the update. Could you also include the test Nevay provided above in this PR?