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

ORTB Blocking module: lack of media type strictness configuration for battr #4006

Open
Net-burst opened this issue Oct 22, 2024 · 2 comments

Comments

@Net-burst
Copy link

There seems to be a design flaw with the blocking module. It was there from the moment the blocking module was initially released, but it became apparent only after we added video and audio support.

The issue lies in the fact that the blocking module doesn't care whether there is a media type object present for the purposes of adding battr. If an account has a configuration for banner/video/audio battr, the module will populate imp[].banner/video/audio.battr even if imp[].banner/video/audio doesn't exist. This would result in the impression potentially having multiple (basically bogus) media types.

Considering that this logic hasn't changed since 2021, I propose adding a control knob for this behavior: hooks.modules.ortb2-blocking.attributes.battr.strict-media-type.

Before injecting a MEDIA_TYPE (banner/video/audio) battr check for strict-media-type:

  • if imp[].MEDIA_TYPE is not null continue with the existing logic;
  • else if imp[].MEDIA_TYPE is null, additionally check for strict-media-type config:
    • if strict-media-type is set to true, skip this MEDIA_TYPE and don't inject battr;
    • if strict-media-type is set to false, apply existing logic: create a new MEDIA_TYPE object inside imp and inject battr.
@justadreamer
Copy link
Contributor

Why introduce another config option and not simply fix it?
i.e.:
if imp[].MEDIA_TYPE is null skip this MEDIA_TYPE and don't inject battr;

@bsardo
Copy link
Collaborator

bsardo commented Oct 23, 2024

Discussed in committee. @bretg do you recall why this module was designed this way? The general opinion here is that this a bug but there is some concern as to whether we should consider this a breaking change such that backwards compatibility should be considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

No branches or pull requests

3 participants