Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logs: Add LogRecordData #3378

Merged
merged 5 commits into from
Jun 18, 2022
Merged

Conversation

CodeBlanch
Copy link
Member

[Goal is to pare down the amount of changes on #3305]

  • Adds struct LogRecordData which will be used in pooling scenarios, by the LogEmitter API, and (potentially) the "Events" API currently under discussion
  • Makes everything on LogRecord settable to unblock users who want to scrub data or otherwise modify it in their pipelines

Public API Changes

namespace OpenTelemetry.Logs
{
    public sealed class LogRecord
    {
        public DateTime Timestamp
        {
            get;
+           set;
        }
        public ActivityTraceId TraceId
        {
            get;
+           set;
        }
        public ActivitySpanId SpanId
        {
            get;
+           set;
        }
        public ActivityTraceFlags TraceFlags
        {
            get;
+           set;
        }
        public string? TraceState
        {
            get;
+           set;
        }
        public string? CategoryName
        {
            get;
+           set;
        }
        public LogLevel LogLevel
        {
            get;
+           set;
        }
        public EventId EventId
        {
            get;
+           set;
        }
        public Exception? Exception
        {
            get;
+           set;
        }
    }
}

@CodeBlanch CodeBlanch requested a review from a team as a code owner June 17, 2022 18:36
Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@codecov
Copy link

codecov bot commented Jun 17, 2022

Codecov Report

Merging #3378 (0e48f44) into main (3c38200) will decrease coverage by 0.07%.
The diff coverage is 65.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3378      +/-   ##
==========================================
- Coverage   85.91%   85.83%   -0.08%     
==========================================
  Files         270      271       +1     
  Lines        9482     9523      +41     
==========================================
+ Hits         8146     8174      +28     
- Misses       1336     1349      +13     
Impacted Files Coverage Δ
src/OpenTelemetry/Logs/LogRecord.cs 84.00% <64.51%> (-12.97%) ⬇️
src/OpenTelemetry/Logs/LogRecordData.cs 65.62% <65.62%> (ø)
...ZPages/Implementation/ZPagesExporterEventSource.cs 56.25% <0.00%> (-6.25%) ⬇️
...Telemetry/Internal/SelfDiagnosticsEventListener.cs 97.65% <0.00%> (+0.78%) ⬆️
...ter.ZPages/Implementation/ZPagesActivityTracker.cs 100.00% <0.00%> (+2.85%) ⬆️
...entation/ExportClient/OtlpGrpcTraceExportClient.cs 50.00% <0.00%> (+14.28%) ⬆️
...xporter.OpenTelemetryProtocol/OtlpTraceExporter.cs 59.09% <0.00%> (+22.72%) ⬆️

/// <param name="activity">Optional <see cref="Activity"/> used to populate context fields.</param>
public LogRecordData(Activity? activity = null)
{
if (activity != null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the ctor here use the static method SetActivityContext to avoid code duplication?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! Sadly, no. This is a weird quick of structs. The compiler doesn't let you access this until the struct has been fully assigned. The reason to call the helper would be to do that assignment so it violates the rule.

        public LogRecordData(Activity? activity = null)
        {
            ref LogRecordData instance = ref this; // Error CS0188  The 'this' object cannot be used before all of its fields have been assigned

            SetActivityContext(ref instance, activity);
        }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i missed that this was a struct. thanks for the explanation!

@CodeBlanch CodeBlanch merged commit 274dc44 into open-telemetry:main Jun 18, 2022
@CodeBlanch CodeBlanch deleted the logrecorddata branch June 18, 2022 04:30
alanwest added a commit that referenced this pull request Jun 27, 2022
* Added Jaeger Propagator to Opentelemetry.Extensions.Propagators (#3309)

* Remove unnecessary bullet in CHANGELOG.md (#3352)

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>

* Fix OTLP test (#3357)

* Show that test is not doing what you might think it does

* More asserts the merrier

* Show this little test that it has potential

* improve test coverage: InMemoryExporter & IDeferredMeterProviderBuilder (#3345)

* [SDK] Circular buffer tweaks + cpu pressure test (#3349)

* CircularBuffer tweaks and cpu pressure test.

* Switch to Volatile.Read.

* Perf tweaks.

* Remove race check in debug after doing more testing.

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>

* Fix event name logic + support null categoryname. (#3359)

* In-memory Exporter: Buffer log scopes (#3360)

* Buffer log scopes when using in-memory exporter.

* CHANGELOG update.

* Code review.

* Tests.

* CHANGELOG tweak.

* SDK: Forward SetParentProvider to children of CompositeProcessor (#3368)

* Examples: Fix ParentProvider not being set on MyFilteringProcessor (#3370)

* Fix ParentProvider not being set on MyFilteringProcessor example.

* Added XML comments.

* Tweak.

* Typo.

* Logs: Add helper ctors & forceflush on OpenTelemetryLoggerProvider (#3364)

* Add helper ctors & forceflush on OpenTelemetryLoggerProvider.

* CHANGELOG update.

* Unit tests.

* Code review.

* Code review.

* Tweak.

* SDK: Nullable annotations for base classes & batch + shims to enable compiler features (#3374)

* Nullable annotations and shims for SDK base classes & batch.

* Target updates.

* Remove System.Collections.Immutable ref.

* ApiCompat attribute exclusions.

* ASPNETCore instrumentation to populate httpflavor tag (#3372)

* improve test coverage: InMemoryExporter SnapshotMetric (#3344)

* Fix AspNetCore metrics to use correct value for http.flavor (#3379)

* Fix AspNetCore metrics to use correct value for http.flavor

* word better

* Logs: Add LogRecordData (#3378)

* Add LogRecordData and hook up to LogRecord.

* CHANGELOG update.

* Code review.

* Remove SetHttpFlavor from Http instrumentations (#3381)

* Asp.Net Core trace instrumentation to populate http schema tag (#3392)

* Try asp.net core tests with inproc server (#3394)

* Dedupe IsPackable (#3398)

* Remove AspNet and AspNet.TelemetryHttpModule instrumentation projects (#3397)

* Handle possible exception when initializing the default service name (#3405)

* HttpClient: Invoke Enrich when SocketException = HostNotFound (#3407)

* Add & use ConfigureResource API. (#3307)

Co-authored-by: tyler jago <ty_bone11@hotmail.com>
Co-authored-by: Robert Pająk <rpajak@splunk.com>
Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
Co-authored-by: Timothy Mothra <tilee@microsoft.com>
Co-authored-by: Mikel Blanchard <mblanchard@macrosssoftware.com>
Co-authored-by: Reiley Yang <reyang@microsoft.com>
Co-authored-by: Utkarsh Umesan Pillai <utpilla@microsoft.com>
Co-authored-by: Christian Neumüller <christian.neumueller@dynatrace.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants