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

exclude file logs where we have otlp logs #945

Closed

Conversation

zeitlinger
Copy link
Member

the duplicate logs are quite confusing

@zeitlinger zeitlinger requested a review from puckpuck as a code owner November 9, 2023 15:51
@zeitlinger zeitlinger requested a review from a team November 9, 2023 15:51
@zeitlinger zeitlinger force-pushed the exclude-duplicate-logs branch from 5c91e77 to bb8887f Compare November 9, 2023 16:28
filelog:
exclude:
- '/var/log/pods/*-otelcol*/opentelemetry-collector/*.log'
# logs are sent as OTLP, so we filter out the file logs
Copy link
Member

Choose a reason for hiding this comment

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

@puckpuck this sounds like a bug with the demo. Is the demo purposefully emitting the logs both as otlp and stdout?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought this is the intended behavior - but I didn't see a definitive answer in https://opentelemetry.io/docs/specs/otel/logs/

Copy link
Member

Choose a reason for hiding this comment

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

@zeitlinger are you adding logs to the demo yourself? I don't believe the demo does logs over otlp in 1.6

Copy link
Member Author

Choose a reason for hiding this comment

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

I have enabled the flag for jvm (adservice, frauddetectionservice), but not for the others (recommendationservice/python, cartservice/dotnet, quoteservice/php)

Copy link
Member Author

Choose a reason for hiding this comment

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

I would actually propose to add the java flag to enable logging as well - it will be enabled by default with 2.0 (of the java agent)

Copy link
Contributor

Choose a reason for hiding this comment

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

The demo has started to add pieces to support logging, but it isn't complete yet. Any logging that services do is likely some defaults from an SDK that is not yet configured.

@JaredTan95
Copy link
Member

pls run make generate-examples to render examples.

Copy link

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@zeitlinger
Copy link
Member Author

@JaredTan95 can you have another look?

@JaredTan95
Copy link
Member

I have a question about this, I didn't seem to see the configuration about collection logs in otel-demo values. If there is no configuration, why do we need to exclude logs? Could you remind me if I missed something :-P

@github-actions github-actions bot removed the Stale label Nov 29, 2023
@zeitlinger
Copy link
Member Author

I have a question about this, I didn't seem to see the configuration about collection logs in otel-demo values. If there is no configuration, why do we need to exclude logs? Could you remind me if I missed something :-P

I've excluded logs for applications that are double reported - once for otlp logs and once for logs from the filelog receiver in the collector.

Copy link

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added Stale and removed Stale labels Dec 14, 2023
Copy link

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 30, 2023
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants